Skip to content

Commit 90ba31d

Browse files
crisbetojelbourn
authored andcommitted
refactor(list): remove redundant focus logic (#12965)
Currently both the regular list and the selection list have focus and blur handlers whose job it is to toggle a focused class. This is redundant, because the same can be achieved with a `:focus` selector, without having to go through change detection.
1 parent 8fd3572 commit 90ba31d

File tree

5 files changed

+3
-89
lines changed

5 files changed

+3
-89
lines changed

src/lib/list/_list-theme.scss

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@
2828

2929
.mat-list-option,
3030
.mat-nav-list .mat-list-item {
31-
&:hover, &.mat-list-item-focus {
31+
&:hover, &:focus {
3232
background: mat-color($background, 'hover');
3333
}
3434
}

src/lib/list/list.spec.ts

Lines changed: 0 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -26,23 +26,6 @@ describe('MatList', () => {
2626
TestBed.compileComponents();
2727
}));
2828

29-
it('should add and remove focus class on focus/blur', () => {
30-
let fixture = TestBed.createComponent(ListWithOneAnchorItem);
31-
fixture.detectChanges();
32-
let listItem = fixture.debugElement.query(By.directive(MatListItem));
33-
let listItemEl = fixture.debugElement.query(By.css('.mat-list-item'));
34-
35-
expect(listItemEl.nativeElement.classList).not.toContain('mat-list-item-focus');
36-
37-
listItem.componentInstance._handleFocus();
38-
fixture.detectChanges();
39-
expect(listItemEl.nativeElement.classList).toContain('mat-list-item-focus');
40-
41-
listItem.componentInstance._handleBlur();
42-
fixture.detectChanges();
43-
expect(listItemEl.nativeElement.classList).not.toContain('mat-list-item-focus');
44-
});
45-
4629
it('should not apply any additional class to a list without lines', () => {
4730
let fixture = TestBed.createComponent(ListWithOneItem);
4831
let listItem = fixture.debugElement.query(By.css('mat-list-item'));

src/lib/list/list.ts

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -99,8 +99,6 @@ export class MatListSubheaderCssMatStyler {}
9999
// @breaking-change 7.0.0 Remove `mat-list-item-avatar` in favor of `mat-list-item-with-avatar`.
100100
'[class.mat-list-item-avatar]': '_avatar || _icon',
101101
'[class.mat-list-item-with-avatar]': '_avatar || _icon',
102-
'(focus)': '_handleFocus()',
103-
'(blur)': '_handleBlur()',
104102
},
105103
inputs: ['disableRipple'],
106104
templateUrl: 'list-item.html',
@@ -132,14 +130,6 @@ export class MatListItem extends _MatListItemMixinBase implements AfterContentIn
132130
return !this._isNavList || this.disableRipple || this._navList.disableRipple;
133131
}
134132

135-
_handleFocus() {
136-
this._element.nativeElement.classList.add('mat-list-item-focus');
137-
}
138-
139-
_handleBlur() {
140-
this._element.nativeElement.classList.remove('mat-list-item-focus');
141-
}
142-
143133
/** Retrieves the DOM element of the component host. */
144134
_getHostElement(): HTMLElement {
145135
return this._element.nativeElement;

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

Lines changed: 0 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -52,21 +52,6 @@ describe('MatSelectionList without forms', () => {
5252
selectionList = fixture.debugElement.query(By.directive(MatSelectionList));
5353
}));
5454

55-
it('should add and remove focus class on focus/blur', () => {
56-
// Use the second list item, because the first one is always disabled.
57-
const listItem = listOptions[1].nativeElement;
58-
59-
expect(listItem.classList).not.toContain('mat-list-item-focus');
60-
61-
dispatchFakeEvent(listItem, 'focus');
62-
fixture.detectChanges();
63-
expect(listItem.className).toContain('mat-list-item-focus');
64-
65-
dispatchFakeEvent(listItem, 'blur');
66-
fixture.detectChanges();
67-
expect(listItem.className).not.toContain('mat-list-item-focus');
68-
});
69-
7055
it('should be able to set a value on a list option', () => {
7156
const optionValues = ['inbox', 'starred', 'sent-mail', 'drafts'];
7257

@@ -519,45 +504,6 @@ describe('MatSelectionList without forms', () => {
519504
});
520505
});
521506

522-
describe('with single option', () => {
523-
let fixture: ComponentFixture<SelectionListWithOnlyOneOption>;
524-
let listOption: DebugElement;
525-
let listItemEl: DebugElement;
526-
527-
beforeEach(async(() => {
528-
TestBed.configureTestingModule({
529-
imports: [MatListModule],
530-
declarations: [
531-
SelectionListWithListOptions,
532-
SelectionListWithCheckboxPositionAfter,
533-
SelectionListWithListDisabled,
534-
SelectionListWithOnlyOneOption
535-
],
536-
});
537-
538-
TestBed.compileComponents();
539-
}));
540-
541-
beforeEach(async(() => {
542-
fixture = TestBed.createComponent(SelectionListWithOnlyOneOption);
543-
listOption = fixture.debugElement.query(By.directive(MatListOption));
544-
listItemEl = fixture.debugElement.query(By.css('.mat-list-item'));
545-
fixture.detectChanges();
546-
}));
547-
548-
it('should be focused when focus on nativeElements', () => {
549-
dispatchFakeEvent(listOption.nativeElement, 'focus');
550-
fixture.detectChanges();
551-
552-
expect(listItemEl.nativeElement.className).toContain('mat-list-item-focus');
553-
554-
dispatchFakeEvent(listOption.nativeElement, 'blur');
555-
fixture.detectChanges();
556-
557-
expect(listItemEl.nativeElement.className).not.toContain('mat-list-item-focus');
558-
});
559-
});
560-
561507
describe('with option disabled', () => {
562508
let fixture: ComponentFixture<SelectionListWithDisabledOption>;
563509
let listOptionEl: HTMLElement;

src/lib/list/selection-list.ts

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -98,9 +98,6 @@ export class MatListOption extends _MatListOptionMixinBase
9898
private _selected = false;
9999
private _disabled = false;
100100

101-
/** Whether the option has focus. */
102-
_hasFocus: boolean = false;
103-
104101
@ContentChild(MatListAvatarCssMatStyler) _avatar: MatListAvatarCssMatStyler;
105102
@ContentChildren(MatLine) _lines: QueryList<MatLine>;
106103

@@ -209,12 +206,10 @@ export class MatListOption extends _MatListOptionMixinBase
209206
}
210207

211208
_handleFocus() {
212-
this._hasFocus = true;
213209
this.selectionList._setFocusedOption(this);
214210
}
215211

216212
_handleBlur() {
217-
this._hasFocus = false;
218213
this.selectionList._onTouched();
219214
}
220215

@@ -389,9 +384,9 @@ export class MatSelectionList extends _MatSelectionListMixinBase implements Focu
389384

390385
/** Removes an option from the selection list and updates the active item. */
391386
_removeOptionFromList(option: MatListOption) {
392-
if (option._hasFocus) {
393-
const optionIndex = this._getOptionIndex(option);
387+
const optionIndex = this._getOptionIndex(option);
394388

389+
if (optionIndex > -1 && this._keyManager.activeItemIndex === optionIndex) {
395390
// Check whether the option is the last item
396391
if (optionIndex > 0) {
397392
this._keyManager.setPreviousItemActive();

0 commit comments

Comments
 (0)