Skip to content

Commit df981ee

Browse files
authored
fix: focus monitor-based styles not working in some cases inside shadow dom (#19422)
A while ago event delegation was implemented for the focus monitor which has some special behavior where it has to bind its events to the closest shadow root in order for them to work correctly. The problem is that a lot of our `FocusMonitor.focus` calls are very early in the component lifecycle which means that might not have been put into the shadow DOM yet. These changes move the `monitor` calls to `ngAfterViewInit` to resolve the issue. Fixes #19414.
1 parent 7e5802a commit df981ee

File tree

24 files changed

+115
-81
lines changed

24 files changed

+115
-81
lines changed

src/cdk/a11y/focus-monitor/focus-monitor.ts

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import {
1818
OnDestroy,
1919
Optional,
2020
Output,
21+
AfterViewInit,
2122
} from '@angular/core';
2223
import {Observable, of as observableOf, Subject, Subscription} from 'rxjs';
2324
import {coerceElement} from '@angular/cdk/coercion';
@@ -562,19 +563,24 @@ function getTarget(event: Event): HTMLElement|null {
562563
@Directive({
563564
selector: '[cdkMonitorElementFocus], [cdkMonitorSubtreeFocus]',
564565
})
565-
export class CdkMonitorFocus implements OnDestroy {
566+
export class CdkMonitorFocus implements AfterViewInit, OnDestroy {
566567
private _monitorSubscription: Subscription;
567568
@Output() cdkFocusChange = new EventEmitter<FocusOrigin>();
568569

569-
constructor(private _elementRef: ElementRef<HTMLElement>, private _focusMonitor: FocusMonitor) {
570+
constructor(private _elementRef: ElementRef<HTMLElement>, private _focusMonitor: FocusMonitor) {}
571+
572+
ngAfterViewInit() {
570573
this._monitorSubscription = this._focusMonitor.monitor(
571-
this._elementRef,
572-
this._elementRef.nativeElement.hasAttribute('cdkMonitorSubtreeFocus'))
573-
.subscribe(origin => this.cdkFocusChange.emit(origin));
574+
this._elementRef,
575+
this._elementRef.nativeElement.hasAttribute('cdkMonitorSubtreeFocus'))
576+
.subscribe(origin => this.cdkFocusChange.emit(origin));
574577
}
575578

576579
ngOnDestroy() {
577580
this._focusMonitor.stopMonitoring(this._elementRef);
578-
this._monitorSubscription.unsubscribe();
581+
582+
if (this._monitorSubscription) {
583+
this._monitorSubscription.unsubscribe();
584+
}
579585
}
580586
}

src/material/button-toggle/button-toggle.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ import {
3030
ViewEncapsulation,
3131
InjectionToken,
3232
Inject,
33+
AfterViewInit,
3334
} from '@angular/core';
3435
import {ControlValueAccessor, NG_VALUE_ACCESSOR} from '@angular/forms';
3536
import {
@@ -394,7 +395,7 @@ const _MatButtonToggleMixinBase: CanDisableRippleCtor & typeof MatButtonToggleBa
394395
'(focus)': 'focus()',
395396
}
396397
})
397-
export class MatButtonToggle extends _MatButtonToggleMixinBase implements OnInit,
398+
export class MatButtonToggle extends _MatButtonToggleMixinBase implements OnInit, AfterViewInit,
398399
CanDisableRipple, OnDestroy {
399400

400401
private _isSingleSelector = false;
@@ -512,7 +513,9 @@ export class MatButtonToggle extends _MatButtonToggleMixinBase implements OnInit
512513
group._syncButtonToggle(this, this._checked);
513514
}
514515
}
516+
}
515517

518+
ngAfterViewInit() {
516519
this._focusMonitor.monitor(this._elementRef, true);
517520
}
518521

src/material/button/button.spec.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,7 @@ describe('MatButton', () => {
7272

7373
it('should be able to focus button with a specific focus origin', () => {
7474
const fixture = TestBed.createComponent(TestApp);
75+
fixture.detectChanges();
7576
const buttonDebugEl = fixture.debugElement.query(By.css('button'));
7677
const buttonInstance = buttonDebugEl.componentInstance as MatButton;
7778

src/material/button/button.ts

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import {
1818
Optional,
1919
Inject,
2020
Input,
21+
AfterViewInit,
2122
} from '@angular/core';
2223
import {
2324
CanColor,
@@ -79,7 +80,7 @@ const _MatButtonMixinBase: CanDisableRippleCtor & CanDisableCtor & CanColorCtor
7980
changeDetection: ChangeDetectionStrategy.OnPush,
8081
})
8182
export class MatButton extends _MatButtonMixinBase
82-
implements OnDestroy, CanDisable, CanColor, CanDisableRipple, FocusableOption {
83+
implements AfterViewInit, OnDestroy, CanDisable, CanColor, CanDisableRipple, FocusableOption {
8384

8485
/** Whether the button is round. */
8586
readonly isRoundButton: boolean = this._hasHostAttributes('mat-fab', 'mat-mini-fab');
@@ -108,13 +109,15 @@ export class MatButton extends _MatButtonMixinBase
108109
// the class is applied to derived classes.
109110
elementRef.nativeElement.classList.add('mat-button-base');
110111

111-
this._focusMonitor.monitor(this._elementRef, true);
112-
113112
if (this.isRoundButton) {
114113
this.color = DEFAULT_ROUND_BUTTON_COLOR;
115114
}
116115
}
117116

117+
ngAfterViewInit() {
118+
this._focusMonitor.monitor(this._elementRef, true);
119+
}
120+
118121
ngOnDestroy() {
119122
this._focusMonitor.stopMonitoring(this._elementRef);
120123
}

src/material/checkbox/checkbox.ts

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -221,7 +221,12 @@ export class MatCheckbox extends _MatCheckboxMixinBase implements ControlValueAc
221221

222222
this.tabIndex = parseInt(tabIndex) || 0;
223223

224-
this._focusMonitor.monitor(elementRef, true).subscribe(focusOrigin => {
224+
// TODO: Remove this after the `_clickAction` parameter is removed as an injection parameter.
225+
this._clickAction = this._clickAction || this._options.clickAction;
226+
}
227+
228+
ngAfterViewInit() {
229+
this._focusMonitor.monitor(this._elementRef, true).subscribe(focusOrigin => {
225230
if (!focusOrigin) {
226231
// When a focused element becomes disabled, the browser *immediately* fires a blur event.
227232
// Angular does not expect events to be raised during change detection, so any state change
@@ -230,16 +235,11 @@ export class MatCheckbox extends _MatCheckboxMixinBase implements ControlValueAc
230235
// telling the form control it has been touched until the next tick.
231236
Promise.resolve().then(() => {
232237
this._onTouched();
233-
_changeDetectorRef.markForCheck();
238+
this._changeDetectorRef.markForCheck();
234239
});
235240
}
236241
});
237242

238-
// TODO: Remove this after the `_clickAction` parameter is removed as an injection parameter.
239-
this._clickAction = this._clickAction || this._options.clickAction;
240-
}
241-
242-
ngAfterViewInit() {
243243
this._syncIndeterminate(this._indeterminate);
244244
}
245245

src/material/expansion/expansion-panel-header.ts

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import {
2020
ViewEncapsulation,
2121
Optional,
2222
Inject,
23+
AfterViewInit,
2324
} from '@angular/core';
2425
import {ANIMATION_MODULE_TYPE} from '@angular/platform-browser/animations';
2526
import {merge, Subscription, EMPTY} from 'rxjs';
@@ -64,7 +65,7 @@ import {MatAccordionTogglePosition} from './accordion-base';
6465
'(keydown)': '_keydown($event)',
6566
},
6667
})
67-
export class MatExpansionPanelHeader implements OnDestroy, FocusableOption {
68+
export class MatExpansionPanelHeader implements AfterViewInit, OnDestroy, FocusableOption {
6869
private _parentChangeSubscription = Subscription.EMPTY;
6970

7071
constructor(
@@ -99,12 +100,6 @@ export class MatExpansionPanelHeader implements OnDestroy, FocusableOption {
99100
.pipe(filter(() => panel._containsFocus()))
100101
.subscribe(() => _focusMonitor.focusVia(_element, 'program'));
101102

102-
_focusMonitor.monitor(_element).subscribe(origin => {
103-
if (origin && panel.accordion) {
104-
panel.accordion._handleHeaderFocus(this);
105-
}
106-
});
107-
108103
if (defaultOptions) {
109104
this.expandedHeight = defaultOptions.expandedHeight;
110105
this.collapsedHeight = defaultOptions.collapsedHeight;
@@ -201,6 +196,14 @@ export class MatExpansionPanelHeader implements OnDestroy, FocusableOption {
201196
this._focusMonitor.focusVia(this._element, origin, options);
202197
}
203198

199+
ngAfterViewInit() {
200+
this._focusMonitor.monitor(this._element).subscribe(origin => {
201+
if (origin && this.panel.accordion) {
202+
this.panel.accordion._handleHeaderFocus(this);
203+
}
204+
});
205+
}
206+
204207
ngOnDestroy() {
205208
this._parentChangeSubscription.unsubscribe();
206209
this._focusMonitor.stopMonitoring(this._element);

src/material/input/input.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,10 +18,10 @@ import {
1818
NgZone,
1919
OnChanges,
2020
OnDestroy,
21-
OnInit,
2221
Optional,
2322
Self,
2423
HostListener,
24+
AfterViewInit,
2525
} from '@angular/core';
2626
import {FormGroupDirective, NgControl, NgForm} from '@angular/forms';
2727
import {
@@ -88,7 +88,7 @@ const _MatInputMixinBase: CanUpdateErrorStateCtor & typeof MatInputBase =
8888
providers: [{provide: MatFormFieldControl, useExisting: MatInput}],
8989
})
9090
export class MatInput extends _MatInputMixinBase implements MatFormFieldControl<any>, OnChanges,
91-
OnDestroy, OnInit, DoCheck, CanUpdateErrorState {
91+
OnDestroy, AfterViewInit, DoCheck, CanUpdateErrorState {
9292
protected _uid = `mat-input-${nextUniqueId++}`;
9393
protected _previousNativeValue: any;
9494
private _inputValueAccessor: {value: any};
@@ -277,7 +277,7 @@ export class MatInput extends _MatInputMixinBase implements MatFormFieldControl<
277277
}
278278
}
279279

280-
ngOnInit() {
280+
ngAfterViewInit() {
281281
if (this._platform.isBrowser) {
282282
this._autofillMonitor.monitor(this._elementRef.nativeElement).subscribe(event => {
283283
this.autofilled = event.isAutofilled;

src/material/menu/menu-item.ts

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import {
1818
Optional,
1919
Input,
2020
HostListener,
21+
AfterViewInit,
2122
} from '@angular/core';
2223
import {
2324
CanDisable, CanDisableCtor,
@@ -57,7 +58,7 @@ const _MatMenuItemMixinBase: CanDisableRippleCtor & CanDisableCtor & typeof MatM
5758
templateUrl: 'menu-item.html',
5859
})
5960
export class MatMenuItem extends _MatMenuItemMixinBase
60-
implements FocusableOption, CanDisable, CanDisableRipple, OnDestroy {
61+
implements FocusableOption, CanDisable, CanDisableRipple, AfterViewInit, OnDestroy {
6162

6263
/** ARIA role for the menu item. */
6364
@Input() role: 'menuitem' | 'menuitemradio' | 'menuitemcheckbox' = 'menuitem';
@@ -85,13 +86,6 @@ export class MatMenuItem extends _MatMenuItemMixinBase
8586
// @breaking-change 8.0.0 make `_focusMonitor` and `document` required params.
8687
super();
8788

88-
if (_focusMonitor) {
89-
// Start monitoring the element so it gets the appropriate focused classes. We want
90-
// to show the focus style for menu items only when the focus was not caused by a
91-
// mouse or touch interaction.
92-
_focusMonitor.monitor(this._elementRef, false);
93-
}
94-
9589
if (_parentMenu && _parentMenu.addItem) {
9690
_parentMenu.addItem(this);
9791
}
@@ -110,6 +104,15 @@ export class MatMenuItem extends _MatMenuItemMixinBase
110104
this._focused.next(this);
111105
}
112106

107+
ngAfterViewInit() {
108+
if (this._focusMonitor) {
109+
// Start monitoring the element so it gets the appropriate focused classes. We want
110+
// to show the focus style for menu items only when the focus was not caused by a
111+
// mouse or touch interaction.
112+
this._focusMonitor.monitor(this._elementRef, false);
113+
}
114+
}
115+
113116
ngOnDestroy() {
114117
if (this._focusMonitor) {
115118
this._focusMonitor.stopMonitoring(this._elementRef);

src/material/slider/slider.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -36,12 +36,12 @@ import {
3636
Inject,
3737
Input,
3838
OnDestroy,
39-
OnInit,
4039
Optional,
4140
Output,
4241
ViewChild,
4342
ViewEncapsulation,
4443
NgZone,
44+
AfterViewInit,
4545
} from '@angular/core';
4646
import {ControlValueAccessor, NG_VALUE_ACCESSOR} from '@angular/forms';
4747
import {
@@ -156,7 +156,7 @@ const _MatSliderMixinBase:
156156
changeDetection: ChangeDetectionStrategy.OnPush,
157157
})
158158
export class MatSlider extends _MatSliderMixinBase
159-
implements ControlValueAccessor, OnDestroy, CanDisable, CanColor, OnInit, HasTabIndex {
159+
implements ControlValueAccessor, OnDestroy, CanDisable, CanColor, AfterViewInit, HasTabIndex {
160160
/** Whether the slider is inverted. */
161161
@Input()
162162
get invert(): boolean { return this._invert; }
@@ -511,7 +511,7 @@ export class MatSlider extends _MatSliderMixinBase
511511
});
512512
}
513513

514-
ngOnInit() {
514+
ngAfterViewInit() {
515515
this._focusMonitor
516516
.monitor(this._elementRef, true)
517517
.subscribe((origin: FocusOrigin) => {

src/material/sort/sort-header.ts

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import {
1818
ViewEncapsulation,
1919
Inject,
2020
ElementRef,
21+
AfterViewInit,
2122
} from '@angular/core';
2223
import {CanDisable, CanDisableCtor, mixinDisabled} from '@angular/material/core';
2324
import {FocusMonitor} from '@angular/cdk/a11y';
@@ -95,7 +96,7 @@ interface MatSortHeaderColumnDef {
9596
]
9697
})
9798
export class MatSortHeader extends _MatSortHeaderMixinBase
98-
implements CanDisable, MatSortable, OnDestroy, OnInit {
99+
implements CanDisable, MatSortable, OnDestroy, OnInit, AfterViewInit {
99100
private _rerenderSubscription: Subscription;
100101

101102
/**
@@ -168,11 +169,6 @@ export class MatSortHeader extends _MatSortHeaderMixinBase
168169

169170
changeDetectorRef.markForCheck();
170171
});
171-
172-
// We use the focus monitor because we also want to style
173-
// things differently based on the focus origin.
174-
_focusMonitor.monitor(_elementRef, true)
175-
.subscribe(origin => this._setIndicatorHintVisible(!!origin));
176172
}
177173

178174
ngOnInit() {
@@ -188,6 +184,13 @@ export class MatSortHeader extends _MatSortHeaderMixinBase
188184
this._sort.register(this);
189185
}
190186

187+
ngAfterViewInit() {
188+
// We use the focus monitor because we also want to style
189+
// things differently based on the focus origin.
190+
this._focusMonitor.monitor(this._elementRef, true)
191+
.subscribe(origin => this._setIndicatorHintVisible(!!origin));
192+
}
193+
191194
ngOnDestroy() {
192195
this._focusMonitor.stopMonitoring(this._elementRef);
193196
this._sort.deregister(this);

src/material/stepper/step-header.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import {
1616
OnDestroy,
1717
ViewEncapsulation,
1818
TemplateRef,
19+
AfterViewInit,
1920
} from '@angular/core';
2021
import {Subscription} from 'rxjs';
2122
import {MatStepLabel} from './step-label';
@@ -35,7 +36,7 @@ import {CdkStepHeader, StepState} from '@angular/cdk/stepper';
3536
encapsulation: ViewEncapsulation.None,
3637
changeDetection: ChangeDetectionStrategy.OnPush,
3738
})
38-
export class MatStepHeader extends CdkStepHeader implements OnDestroy {
39+
export class MatStepHeader extends CdkStepHeader implements AfterViewInit, OnDestroy {
3940
private _intlSubscription: Subscription;
4041

4142
/** State of the given step. */
@@ -71,10 +72,13 @@ export class MatStepHeader extends CdkStepHeader implements OnDestroy {
7172
_elementRef: ElementRef<HTMLElement>,
7273
changeDetectorRef: ChangeDetectorRef) {
7374
super(_elementRef);
74-
_focusMonitor.monitor(_elementRef, true);
7575
this._intlSubscription = _intl.changes.subscribe(() => changeDetectorRef.markForCheck());
7676
}
7777

78+
ngAfterViewInit() {
79+
this._focusMonitor.monitor(this._elementRef, true);
80+
}
81+
7882
ngOnDestroy() {
7983
this._intlSubscription.unsubscribe();
8084
this._focusMonitor.stopMonitoring(this._elementRef);

0 commit comments

Comments
 (0)