From 2c6a50b7a54e078c6cc3fd1fc66d1fa25e7e8123 Mon Sep 17 00:00:00 2001 From: Kristiyan Kostadinov Date: Thu, 26 Sep 2024 09:11:19 +0200 Subject: [PATCH 1/2] fix(material/chips): chip grid not re-focusing first item There was some logic that assumed that if a chip is the active item in the key manager, it must have focus. That's incorrect since the active item isn't reset on blur. This prevented the chip grid from re-focusing the first chip when the user tabs into it a second time. These changes add a `focus` call whenever the grid receives focus. Fixes #29785. --- src/material/chips/chip-grid.ts | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/material/chips/chip-grid.ts b/src/material/chips/chip-grid.ts index d3d84d469bbc..8083c3c6b899 100644 --- a/src/material/chips/chip-grid.ts +++ b/src/material/chips/chip-grid.ts @@ -336,8 +336,14 @@ export class MatChipGrid // Delay until the next tick, because this can cause a "changed after checked" // error if the input does something on focus (e.g. opens an autocomplete). Promise.resolve().then(() => this._chipInput.focus()); - } else if (this._chips.length && this._keyManager.activeItemIndex !== 0) { - this._keyManager.setFirstItemActive(); + } else { + const activeItem = this._keyManager.activeItem; + + if (activeItem) { + activeItem.focus(); + } else { + this._keyManager.setFirstItemActive(); + } } this.stateChanges.next(); From 667ecbff094660a1d034013dfeb3d1ffe7fed347 Mon Sep 17 00:00:00 2001 From: Kristiyan Kostadinov Date: Thu, 26 Sep 2024 09:12:28 +0200 Subject: [PATCH 2/2] fix(material/chips): focus escape not working consistently Fixes that the chip grid was using change detection to allow focus to escape on tab presses. That's unreliable, because change detection might not be executed immediately. These changes fix the issue by changing the DOM node directly. --- src/material/chips/chip-grid.spec.ts | 12 ++++++------ src/material/chips/chip-listbox.spec.ts | 10 +++++----- src/material/chips/chip-set.ts | 16 ++++++++-------- 3 files changed, 19 insertions(+), 19 deletions(-) diff --git a/src/material/chips/chip-grid.spec.ts b/src/material/chips/chip-grid.spec.ts index c78f3771818f..c6c177552da4 100644 --- a/src/material/chips/chip-grid.spec.ts +++ b/src/material/chips/chip-grid.spec.ts @@ -396,23 +396,23 @@ describe('MatChipGrid', () => { dispatchKeyboardEvent(firstNativeChip, 'keydown', TAB); - expect(chipGridInstance.tabIndex) + expect(chipGridNativeElement.tabIndex) .withContext('Expected tabIndex to be set to -1 temporarily.') .toBe(-1); flush(); - expect(chipGridInstance.tabIndex) + expect(chipGridNativeElement.tabIndex) .withContext('Expected tabIndex to be reset back to 0') .toBe(0); })); - it(`should use user defined tabIndex`, fakeAsync(() => { + it('should use user defined tabIndex', fakeAsync(() => { chipGridInstance.tabIndex = 4; fixture.changeDetectorRef.markForCheck(); fixture.detectChanges(); - expect(chipGridInstance.tabIndex) + expect(chipGridNativeElement.tabIndex) .withContext('Expected tabIndex to be set to user defined value 4.') .toBe(4); @@ -420,13 +420,13 @@ describe('MatChipGrid', () => { let firstNativeChip = nativeChips[0] as HTMLElement; dispatchKeyboardEvent(firstNativeChip, 'keydown', TAB); - expect(chipGridInstance.tabIndex) + expect(chipGridNativeElement.tabIndex) .withContext('Expected tabIndex to be set to -1 temporarily.') .toBe(-1); flush(); - expect(chipGridInstance.tabIndex) + expect(chipGridNativeElement.tabIndex) .withContext('Expected tabIndex to be reset back to 4') .toBe(4); })); diff --git a/src/material/chips/chip-listbox.spec.ts b/src/material/chips/chip-listbox.spec.ts index 6a97e749de7b..b87a91e7a7b9 100644 --- a/src/material/chips/chip-listbox.spec.ts +++ b/src/material/chips/chip-listbox.spec.ts @@ -367,13 +367,13 @@ describe('MatChipListbox', () => { it('should allow focus to escape when tabbing away', fakeAsync(() => { dispatchKeyboardEvent(chipListboxNativeElement, 'keydown', TAB); - expect(chipListboxInstance.tabIndex) + expect(chipListboxNativeElement.tabIndex) .withContext('Expected tabIndex to be set to -1 temporarily.') .toBe(-1); flush(); - expect(chipListboxInstance.tabIndex) + expect(chipListboxNativeElement.tabIndex) .withContext('Expected tabIndex to be reset back to 0') .toBe(0); })); @@ -384,19 +384,19 @@ describe('MatChipListbox', () => { fixture.detectChanges(); - expect(chipListboxInstance.tabIndex) + expect(chipListboxNativeElement.tabIndex) .withContext('Expected tabIndex to be set to user defined value 4.') .toBe(4); dispatchKeyboardEvent(chipListboxNativeElement, 'keydown', TAB); - expect(chipListboxInstance.tabIndex) + expect(chipListboxNativeElement.tabIndex) .withContext('Expected tabIndex to be set to -1 temporarily.') .toBe(-1); flush(); - expect(chipListboxInstance.tabIndex) + expect(chipListboxNativeElement.tabIndex) .withContext('Expected tabIndex to be reset back to 4') .toBe(4); })); diff --git a/src/material/chips/chip-set.ts b/src/material/chips/chip-set.ts index cb3f233ba8cd..d31cd06bfd38 100644 --- a/src/material/chips/chip-set.ts +++ b/src/material/chips/chip-set.ts @@ -192,17 +192,17 @@ export class MatChipSet implements AfterViewInit, OnDestroy { * it back to the first chip, creating a focus trap, if it user tries to tab away. */ protected _allowFocusEscape() { - if (this.tabIndex !== -1) { - const previousTabIndex = this.tabIndex; - this.tabIndex = -1; - this._changeDetectorRef.markForCheck(); + const previous = this._elementRef.nativeElement.tabIndex; + + if (previous !== -1) { + // Set the tabindex directly on the element, instead of going through + // the data binding, because we aren't guaranteed that change detection + // will run quickly enough to allow focus to escape. + this._elementRef.nativeElement.tabIndex = -1; // Note that this needs to be a `setTimeout`, because a `Promise.resolve` // doesn't allow enough time for the focus to escape. - setTimeout(() => { - this.tabIndex = previousTabIndex; - this._changeDetectorRef.markForCheck(); - }); + setTimeout(() => (this._elementRef.nativeElement.tabIndex = previous)); } }