Skip to content

Commit 17317fb

Browse files
lskramarovlskramarov
authored
fix(filter-bar): restore keyboard focus ring on pipe trigger after selection (#DS-3829) (#1694)
1 parent 46c0eb9 commit 17317fb

18 files changed

Lines changed: 392 additions & 36 deletions

‎packages/components/filter-bar/pipe-add.spec.ts‎

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { ChangeDetectorRef, Component, DebugElement, inject } from '@angular/cor
22
import { ComponentFixture, fakeAsync, flush, TestBed } from '@angular/core/testing';
33
import { By } from '@angular/platform-browser';
44
import { NoopAnimationsModule } from '@angular/platform-browser/animations';
5+
import { createKeyboardEvent, dispatchEvent, ENTER, SPACE } from '@koobiq/components/core';
56
import {
67
KbqFilter,
78
KbqFilterBar,
@@ -413,6 +414,103 @@ describe('KbqPipeAdd', () => {
413414
}));
414415
});
415416

417+
describe('keyboard (Enter/Space)', () => {
418+
beforeEach(() => {
419+
fixture = TestBed.createComponent(TestComponent);
420+
filterBarDebugElement = fixture.debugElement.query(By.directive(KbqFilterBar));
421+
fixture.detectChanges();
422+
});
423+
424+
const pressEnterOnFirstOption = () => {
425+
const option = document.querySelectorAll('.kbq-option')[0] as HTMLElement;
426+
427+
dispatchEvent(option, createKeyboardEvent('keydown', ENTER, undefined, 'Enter'));
428+
};
429+
430+
const pressSpaceOnFirstOption = () => {
431+
const option = document.querySelectorAll('.kbq-option')[0] as HTMLElement;
432+
433+
dispatchEvent(option, createKeyboardEvent('keydown', SPACE, undefined, ' '));
434+
};
435+
436+
it('should add a pipe when Enter is pressed on a template option', fakeAsync(() => {
437+
const filterBar = getFilterBar();
438+
const pipeAdd = getPipeAdd();
439+
440+
pipeAdd.select().open();
441+
flush();
442+
fixture.detectChanges();
443+
444+
pressEnterOnFirstOption();
445+
flush();
446+
fixture.detectChanges();
447+
448+
expect(filterBar.filter!.pipes.length).toBe(1);
449+
expect(filterBar.filter!.pipes[0].id).toBe(PIPE_TEMPLATE_ID_1);
450+
}));
451+
452+
it('should add a pipe when Space is pressed on a template option', fakeAsync(() => {
453+
const filterBar = getFilterBar();
454+
const pipeAdd = getPipeAdd();
455+
456+
pipeAdd.select().open();
457+
flush();
458+
fixture.detectChanges();
459+
460+
pressSpaceOnFirstOption();
461+
flush();
462+
fixture.detectChanges();
463+
464+
expect(filterBar.filter!.pipes.length).toBe(1);
465+
expect(filterBar.filter!.pipes[0].id).toBe(PIPE_TEMPLATE_ID_1);
466+
}));
467+
468+
it('should emit onAddPipe when Enter is pressed on a template option', fakeAsync(() => {
469+
const pipeAdd = getPipeAdd();
470+
const spy = jest.fn();
471+
472+
pipeAdd.onAddPipe.subscribe(spy);
473+
474+
pipeAdd.select().open();
475+
flush();
476+
fixture.detectChanges();
477+
478+
pressEnterOnFirstOption();
479+
flush();
480+
fixture.detectChanges();
481+
482+
expect(spy).toHaveBeenCalledWith(expect.objectContaining({ id: PIPE_TEMPLATE_ID_1 }));
483+
}));
484+
485+
it('should call filterBar.openPipe.next when Enter is pressed on an already-added option', fakeAsync(() => {
486+
const filterBar = getFilterBar();
487+
const pipeAdd = getPipeAdd();
488+
const openPipeSpy = jest.spyOn(filterBar.openPipe, 'next');
489+
490+
// First Enter — add the pipe
491+
pipeAdd.select().open();
492+
flush();
493+
fixture.detectChanges();
494+
495+
pressEnterOnFirstOption();
496+
flush();
497+
fixture.detectChanges();
498+
499+
// Second Enter — option is now selected, should trigger openPipe
500+
pipeAdd.select().open();
501+
flush();
502+
fixture.detectChanges();
503+
504+
openPipeSpy.mockClear();
505+
506+
pressEnterOnFirstOption();
507+
flush();
508+
fixture.detectChanges();
509+
510+
expect(openPipeSpy).toHaveBeenCalledWith(PIPE_TEMPLATE_ID_1);
511+
}));
512+
});
513+
416514
describe('compareWith', () => {
417515
beforeEach(() => {
418516
fixture = TestBed.createComponent(TestComponent);

‎packages/components/filter-bar/pipe-add.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,8 @@ import { getId } from './pipes/base-pipe';
4040
[value]="template"
4141
[showCheckbox]="false"
4242
(click)="addPipeFromTemplate(option)"
43+
(keydown.enter)="addPipeFromTemplate(option)"
44+
(keydown.space)="addPipeFromTemplate(option)"
4345
>
4446
{{ template.name }}
4547
</kbq-option>

‎packages/components/filter-bar/pipes/base-pipe.ts‎

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
import { FocusMonitor, FocusOrigin, InputModalityDetector } from '@angular/cdk/a11y';
2+
import { DOCUMENT } from '@angular/common';
13
import {
24
afterNextRender,
35
AfterViewInit,
@@ -47,6 +49,15 @@ export abstract class KbqBasePipe<V> implements AfterViewInit {
4749
protected readonly changeDetectorRef = inject(ChangeDetectorRef);
4850
/** @docs-private */
4951
protected readonly destroyRef = inject(DestroyRef);
52+
/** @docs-private */
53+
protected readonly focusMonitor = inject(FocusMonitor);
54+
/** @docs-private */
55+
protected readonly elementRef = inject<ElementRef<HTMLElement>>(ElementRef);
56+
private readonly inputModalityDetector = inject(InputModalityDetector);
57+
private readonly document = inject(DOCUMENT);
58+
59+
/** Last known focus origin within the pipe. Used to preserve the keyboard focus ring on restore. */
60+
private focusOrigin: FocusOrigin = null;
5061

5162
/** values to select from the pipe template */
5263
protected values;
@@ -89,6 +100,18 @@ export abstract class KbqBasePipe<V> implements AfterViewInit {
89100

90101
this.filterBar?.internalTemplatesChanges.pipe(takeUntilDestroyed()).subscribe(this.updateTemplates);
91102

103+
// Track the focus origin so the trigger's keyboard focus ring can be restored after
104+
// a value is chosen. `checkChildren` captures focus on the inner trigger button.
105+
this.focusMonitor
106+
.monitor(this.elementRef, true)
107+
.pipe(
108+
filter((origin) => !!origin),
109+
takeUntilDestroyed()
110+
)
111+
.subscribe((origin) => (this.focusOrigin = origin));
112+
113+
this.destroyRef.onDestroy(() => this.focusMonitor.stopMonitoring(this.elementRef));
114+
92115
afterNextRender(() => {
93116
this.isMac = isMac();
94117
});
@@ -146,6 +169,41 @@ export abstract class KbqBasePipe<V> implements AfterViewInit {
146169
this.filterBar?.onChangePipe.next(this.data);
147170
}
148171

172+
/**
173+
* Restores focus to the pipe's trigger button after a value is chosen or the panel closes.
174+
* Focuses via {@link FocusMonitor} with the captured origin so a keyboard-driven interaction
175+
* keeps its focus ring, while a mouse-driven one does not.
176+
*
177+
* @docs-private
178+
*/
179+
protected restoreTriggerFocus(): void {
180+
const active = this.document.activeElement;
181+
182+
// The panel may have been closed by moving focus to another interactive element
183+
// (e.g. an outside click) — stealing focus back would break the user's intent.
184+
// Focus inside the pipe or inside a not-yet-detached overlay is fine to take over.
185+
if (
186+
active &&
187+
active !== this.document.body &&
188+
!this.elementRef.nativeElement.contains(active) &&
189+
!active.closest('.cdk-overlay-container')
190+
) {
191+
return;
192+
}
193+
194+
const trigger = this.elementRef.nativeElement.querySelector<HTMLElement>(
195+
'button:not(.kbq-pipe__remove-button)'
196+
);
197+
198+
// A pipe opened right after being added from pipe-add never received focus itself,
199+
// so `focusOrigin` is unknown there — fall back to the detected input modality.
200+
const origin = this.focusOrigin ?? this.inputModalityDetector.mostRecentModality ?? 'program';
201+
202+
if (trigger) {
203+
this.focusMonitor.focusVia(trigger, origin);
204+
}
205+
}
206+
149207
/** @docs-private */
150208
abstract open(): void;
151209
}

‎packages/components/filter-bar/pipes/pipe-date.spec.ts‎

Lines changed: 28 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { FocusMonitor } from '@angular/cdk/a11y';
12
import { OverlayContainer } from '@angular/cdk/overlay';
23
import { ChangeDetectorRef, Component, DebugElement, inject, LOCALE_ID } from '@angular/core';
34
import { ComponentFixture, fakeAsync, flush, TestBed } from '@angular/core/testing';
@@ -393,11 +394,12 @@ describe('KbqPipeDateComponent', () => {
393394
setupSinglePipe({ value: null });
394395
});
395396

396-
it('should set data.value, emit onChangePipe and hide popover', () => {
397+
it('should set data.value, emit onChangePipe, hide popover and restore focus', fakeAsync(() => {
397398
const component = getPipeComponent();
398399
const filterBar = getFilterBar();
399400
const spy = jest.fn();
400401
const hide = jest.fn();
402+
const focusViaSpy = jest.spyOn(TestBed.inject(FocusMonitor), 'focusVia');
401403

402404
asInternal(component).popover = () => ({ hide });
403405
filterBar.onChangePipe.subscribe(spy);
@@ -407,20 +409,25 @@ describe('KbqPipeDateComponent', () => {
407409
expect(component.data.value).toEqual({ name: 'test', start: '', end: '' });
408410
expect(spy).toHaveBeenCalledWith(component.data);
409411
expect(hide).toHaveBeenCalled();
410-
});
412+
413+
flush();
414+
415+
expect(focusViaSpy).toHaveBeenCalledWith(expect.any(HTMLButtonElement), expect.anything());
416+
}));
411417
});
412418

413419
describe('onApplyPeriod', () => {
414420
beforeEach(() => {
415421
setupSinglePipe({ value: null });
416422
});
417423

418-
it('should save custom period as ISO strings and emit onChangePipe', () => {
424+
it('should save custom period as ISO strings, emit onChangePipe and restore focus', fakeAsync(() => {
419425
const component = getPipeComponent();
420426
const internal = asInternal(component);
421427
const filterBar = getFilterBar();
422428
const spy = jest.fn();
423429
const hide = jest.fn();
430+
const focusViaSpy = jest.spyOn(TestBed.inject(FocusMonitor), 'focusVia');
424431
const start = adapter.today().minus({ days: 5 });
425432
const end = adapter.today();
426433

@@ -439,7 +446,11 @@ describe('KbqPipeDateComponent', () => {
439446
});
440447
expect(spy).toHaveBeenCalledWith(component.data);
441448
expect(hide).toHaveBeenCalled();
442-
});
449+
450+
flush();
451+
452+
expect(focusViaSpy).toHaveBeenCalledWith(expect.any(HTMLButtonElement), expect.anything());
453+
}));
443454
});
444455

445456
describe('disabled', () => {
@@ -573,6 +584,19 @@ describe('KbqPipeDateComponent', () => {
573584

574585
expect(spy).toHaveBeenCalledWith(component.data);
575586
});
587+
588+
it('should focus the period list when the popover opens so Enter can pick a preset', fakeAsync(() => {
589+
const component = getPipeComponent();
590+
const focus = jest.fn();
591+
592+
asInternal(component).isListMode = true;
593+
asInternal(component).listSelection = () => ({ focus });
594+
595+
component.popover().visibleChange.emit(true);
596+
flush();
597+
598+
expect(focus).toHaveBeenCalled();
599+
}));
576600
});
577601

578602
describe('onClear', () => {

‎packages/components/filter-bar/pipes/pipe-date.ts‎

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ import { KbqListModule, KbqListSelection } from '@koobiq/components/list';
1818
import { KbqPopoverModule, KbqPopoverTrigger } from '@koobiq/components/popover';
1919
import { KbqTimepickerModule } from '@koobiq/components/timepicker';
2020
import { KbqTitleModule } from '@koobiq/components/title';
21-
import { filter } from 'rxjs/operators';
2221
import { KbqDateTimeValue } from '../filter-bar.types';
2322
import { KbqBasePipe } from './base-pipe';
2423
import { KbqPipeButton } from './pipe-button';
@@ -133,19 +132,28 @@ export class KbqPipeDateComponent<D> extends KbqBasePipe<KbqDateTimeValue> imple
133132
/** @docs-private */
134133
readonly popover = viewChild.required<KbqPopoverTrigger>('popover');
135134
/** @docs-private */
136-
listSelection = viewChild.required('listSelection', { read: KbqListSelection });
135+
// Optional: the list only exists while the popover is open in list mode, and it is read
136+
// in deferred callbacks that may fire after the popover has already closed.
137+
listSelection = viewChild('listSelection', { read: KbqListSelection });
137138
/** @docs-private */
138139
returnButton = viewChild.required('returnButton', { read: KbqButton });
139140

140141
override ngAfterViewInit() {
141142
super.ngAfterViewInit();
142143

143144
this.popover()
144-
.visibleChange.pipe(
145-
filter((visible) => !visible),
146-
takeUntilDestroyed(this.destroyRef)
147-
)
148-
.subscribe(() => this.filterBar?.onClosePipe.emit(this.data));
145+
.visibleChange.pipe(takeUntilDestroyed(this.destroyRef))
146+
.subscribe((visible) => {
147+
if (visible) {
148+
// Move keyboard focus onto the period list so Enter selects a preset
149+
// instead of the focus trap landing on the "custom period" row.
150+
if (this.isListMode) {
151+
setTimeout(() => this.listSelection()?.focus());
152+
}
153+
} else {
154+
this.filterBar?.onClosePipe.emit(this.data);
155+
}
156+
});
149157
}
150158

151159
/** keydown handler
@@ -170,6 +178,8 @@ export class KbqPipeDateComponent<D> extends KbqBasePipe<KbqDateTimeValue> imple
170178
this.filterBar?.onChangePipe.next(this.data);
171179

172180
this.popover().hide();
181+
182+
setTimeout(() => this.restoreTriggerFocus());
173183
}
174184

175185
onSelect(item: KbqDateTimeValue) {
@@ -179,6 +189,8 @@ export class KbqPipeDateComponent<D> extends KbqBasePipe<KbqDateTimeValue> imple
179189
this.filterBar?.onChangePipe.next(this.data);
180190

181191
this.popover().hide();
192+
193+
setTimeout(() => this.restoreTriggerFocus());
182194
}
183195

184196
showPeriod() {
@@ -197,7 +209,7 @@ export class KbqPipeDateComponent<D> extends KbqBasePipe<KbqDateTimeValue> imple
197209
showList() {
198210
this.isListMode = true;
199211

200-
setTimeout(() => this.listSelection().focus());
212+
setTimeout(() => this.listSelection()?.focus());
201213
this.popover().updatePosition(true);
202214
}
203215

0 commit comments

Comments
 (0)