Skip to content

Commit 1c307c8

Browse files
devversionandrewseguin
authored andcommitted
fix(button): focus method does not respect specified origin (#17183)
It looks like `MatButton` at some point has been refactored to implement the `FocusableOption` interface so that it can be used easily in the `FocusKeyManager`. Currently since the `origin` parameter in the `focus` method is not respected, the `FocusKeyManager#setFocusOrigin` method is a noop for button elements. Additionally consumers of `MatButton` who pass in a specific `FocusOrigin` when calling the `focus` method will be surprised that the `origin` is simply ignored (even though it's part of the public method signature; just prefixed with an underscore). We should just respect the focus origin to make the method less confusing and to properly implement the `FocusableOption`. Fixes #17174
1 parent b0c69dd commit 1c307c8

File tree

3 files changed

+16
-5
lines changed

3 files changed

+16
-5
lines changed

src/material/button/button.spec.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,19 @@ describe('MatButton', () => {
7070
expect(buttonDebugElement.nativeElement.classList.contains('custom-class')).toBe(true);
7171
});
7272

73+
it('should be able to focus button with a specific focus origin', () => {
74+
const fixture = TestBed.createComponent(TestApp);
75+
const buttonDebugEl = fixture.debugElement.query(By.css('button'));
76+
const buttonInstance = buttonDebugEl.componentInstance as MatButton;
77+
78+
expect(buttonDebugEl.nativeElement.classList).not.toContain('cdk-focused');
79+
80+
buttonInstance.focus('touch');
81+
82+
expect(buttonDebugEl.nativeElement.classList).toContain('cdk-focused');
83+
expect(buttonDebugEl.nativeElement.classList).toContain('cdk-touch-focused');
84+
});
85+
7386
describe('button[mat-fab]', () => {
7487
it('should have accent palette by default', () => {
7588
const fixture = TestBed.createComponent(TestApp);

src/material/button/button.ts

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -119,10 +119,8 @@ export class MatButton extends _MatButtonMixinBase
119119
}
120120

121121
/** Focuses the button. */
122-
focus(_origin?: FocusOrigin, options?: FocusOptions): void {
123-
// Note that we aren't using `_origin`, but we need to keep it because some internal consumers
124-
// use `MatButton` in a `FocusKeyManager` and we need it to match `FocusableOption`.
125-
this._getHostElement().focus(options);
122+
focus(origin: FocusOrigin = 'program', options?: FocusOptions): void {
123+
this._focusMonitor.focusVia(this._getHostElement(), origin, options);
126124
}
127125

128126
_getHostElement() {

tools/public_api_guard/material/button.d.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ export declare class MatButton extends _MatButtonMixinBase implements OnDestroy,
1313
_getHostElement(): any;
1414
_hasHostAttributes(...attributes: string[]): boolean;
1515
_isRippleDisabled(): boolean;
16-
focus(_origin?: FocusOrigin, options?: FocusOptions): void;
16+
focus(origin?: FocusOrigin, options?: FocusOptions): void;
1717
ngOnDestroy(): void;
1818
}
1919

0 commit comments

Comments
 (0)