Skip to content

Commit f3b7798

Browse files
crisbetojosephperrott
authored andcommitted
fix(list): selection list moving focus when an option is destroyed (#13531)
1 parent 1280c7d commit f3b7798

File tree

2 files changed

+40
-5
lines changed

2 files changed

+40
-5
lines changed

src/lib/list/selection-list.spec.ts

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -214,6 +214,7 @@ describe('MatSelectionList without forms', () => {
214214
it('should restore focus if active option is destroyed', () => {
215215
const manager = selectionList.componentInstance._keyManager;
216216

217+
spyOn(listOptions[2].componentInstance, 'focus').and.callThrough();
217218
listOptions[3].componentInstance._handleFocus();
218219

219220
expect(manager.activeItemIndex).toBe(3);
@@ -222,8 +223,28 @@ describe('MatSelectionList without forms', () => {
222223
fixture.detectChanges();
223224

224225
expect(manager.activeItemIndex).toBe(2);
226+
expect(listOptions[2].componentInstance.focus).toHaveBeenCalled();
225227
});
226228

229+
it('should not attempt to focus the next option when the destroyed option was not focused',
230+
() => {
231+
const manager = selectionList.componentInstance._keyManager;
232+
233+
// Focus and blur the option to move the active item index.
234+
listOptions[3].componentInstance._handleFocus();
235+
listOptions[3].componentInstance._handleBlur();
236+
237+
spyOn(listOptions[2].componentInstance, 'focus').and.callThrough();
238+
239+
expect(manager.activeItemIndex).toBe(3);
240+
241+
fixture.componentInstance.showLastOption = false;
242+
fixture.detectChanges();
243+
244+
expect(manager.activeItemIndex).toBe(2);
245+
expect(listOptions[2].componentInstance.focus).not.toHaveBeenCalled();
246+
});
247+
227248
it('should focus previous item when press UP ARROW', () => {
228249
let testListItem = listOptions[2].nativeElement as HTMLElement;
229250
let UP_EVENT: KeyboardEvent =

src/lib/list/selection-list.ts

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,7 @@ export class MatListOption extends _MatListOptionMixinBase
9898

9999
private _selected = false;
100100
private _disabled = false;
101+
private _hasFocus = false;
101102

102103
@ContentChild(MatListAvatarCssMatStyler) _avatar: MatListAvatarCssMatStyler;
103104
@ContentChild(MatListIconCssMatStyler) _icon: MatListIconCssMatStyler;
@@ -172,7 +173,13 @@ export class MatListOption extends _MatListOptionMixinBase
172173
Promise.resolve().then(() => this.selected = false);
173174
}
174175

175-
this.selectionList._removeOptionFromList(this);
176+
const hadFocus = this._hasFocus;
177+
const newActiveItem = this.selectionList._removeOptionFromList(this);
178+
179+
// Only move focus if this option was focused at the time it was destroyed.
180+
if (hadFocus && newActiveItem) {
181+
newActiveItem.focus();
182+
}
176183
}
177184

178185
/** Toggles the selection state of the option. */
@@ -209,10 +216,12 @@ export class MatListOption extends _MatListOptionMixinBase
209216

210217
_handleFocus() {
211218
this.selectionList._setFocusedOption(this);
219+
this._hasFocus = true;
212220
}
213221

214222
_handleBlur() {
215223
this.selectionList._onTouched();
224+
this._hasFocus = false;
216225
}
217226

218227
/** Retrieves the DOM element of the component host. */
@@ -385,18 +394,23 @@ export class MatSelectionList extends _MatSelectionListMixinBase implements Focu
385394
this._keyManager.updateActiveItemIndex(this._getOptionIndex(option));
386395
}
387396

388-
/** Removes an option from the selection list and updates the active item. */
389-
_removeOptionFromList(option: MatListOption) {
397+
/**
398+
* Removes an option from the selection list and updates the active item.
399+
* @returns Currently-active item.
400+
*/
401+
_removeOptionFromList(option: MatListOption): MatListOption | null {
390402
const optionIndex = this._getOptionIndex(option);
391403

392404
if (optionIndex > -1 && this._keyManager.activeItemIndex === optionIndex) {
393405
// Check whether the option is the last item
394406
if (optionIndex > 0) {
395-
this._keyManager.setPreviousItemActive();
407+
this._keyManager.updateActiveItemIndex(optionIndex - 1);
396408
} else if (optionIndex === 0 && this.options.length > 1) {
397-
this._keyManager.setNextItemActive();
409+
this._keyManager.updateActiveItemIndex(Math.min(optionIndex + 1, this.options.length - 1));
398410
}
399411
}
412+
413+
return this._keyManager.activeItem;
400414
}
401415

402416
/** Passes relevant key presses to our key manager. */

0 commit comments

Comments
 (0)