Skip to content

fix(cdk/a11y): correctly detect focus from input label #25232

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 11, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 53 additions & 0 deletions src/cdk/a11y/focus-monitor/focus-monitor.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -778,6 +778,51 @@ describe('FocusMonitor observable stream', () => {
}));
});

describe('FocusMonitor input label detection', () => {
let fixture: ComponentFixture<CheckboxWithLabel>;
let inputElement: HTMLElement;
let labelElement: HTMLElement;
let focusMonitor: FocusMonitor;

beforeEach(() => {
TestBed.configureTestingModule({
imports: [A11yModule],
declarations: [CheckboxWithLabel],
}).compileComponents();
});

beforeEach(inject([FocusMonitor], (fm: FocusMonitor) => {
fixture = TestBed.createComponent(CheckboxWithLabel);
focusMonitor = fm;
fixture.detectChanges();
inputElement = fixture.nativeElement.querySelector('input');
labelElement = fixture.nativeElement.querySelector('label');
patchElementFocus(inputElement);
}));

it('should detect label click focus as `mouse`', fakeAsync(() => {
const spy = jasmine.createSpy('monitor spy');
focusMonitor.monitor(inputElement).subscribe(spy);
expect(spy).not.toHaveBeenCalled();

// Unlike most focus, focus from labels moves to the connected input on click rather than
// `mousedown`. To simulate it we have to dispatch both `mousedown` and `click` so the
// modality detector will pick it up.
dispatchMouseEvent(labelElement, 'mousedown');
labelElement.click();
fixture.detectChanges();
flush();

// The programmatic click from above won't move focus so we have to focus the input ourselves.
inputElement.focus();
fixture.detectChanges();
tick();

expect(inputElement.classList).toContain('cdk-mouse-focused');
expect(spy.calls.mostRecent()?.args[0]).toBe('mouse');
}));
});

@Component({
template: `<div class="parent"><button>focus me!</button></div>`,
})
Expand Down Expand Up @@ -809,3 +854,11 @@ class ComplexComponentWithMonitorSubtreeFocusAndMonitorElementFocus {}
template: `<ng-container cdkMonitorElementFocus></ng-container>`,
})
class FocusMonitorOnCommentNode {}

@Component({
template: `
<label for="test-checkbox">Check me</label>
<input id="test-checkbox" type="checkbox">
`,
})
class CheckboxWithLabel {}
55 changes: 52 additions & 3 deletions src/cdk/a11y/focus-monitor/focus-monitor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -160,11 +160,14 @@ export class FocusMonitor implements OnDestroy {
*/
private _rootNodeFocusAndBlurListener = (event: Event) => {
const target = _getEventTarget<HTMLElement>(event);
const handler = event.type === 'focus' ? this._onFocus : this._onBlur;

// We need to walk up the ancestor chain in order to support `checkChildren`.
for (let element = target; element; element = element.parentElement) {
handler.call(this, event as FocusEvent, element);
if (event.type === 'focus') {
this._onFocus(event as FocusEvent, element);
} else {
this._onBlur(event as FocusEvent, element);
}
}
};

Expand Down Expand Up @@ -328,7 +331,19 @@ export class FocusMonitor implements OnDestroy {
// events).
//
// Because we can't distinguish between these two cases, we default to setting `program`.
return this._windowFocused && this._lastFocusOrigin ? this._lastFocusOrigin : 'program';
if (this._windowFocused && this._lastFocusOrigin) {
return this._lastFocusOrigin;
}

// If the interaction is coming from an input label, we consider it a mouse interactions.
// This is a special case where focus moves on `click`, rather than `mousedown` which breaks
// our detection, because all our assumptions are for `mousedown`. We need to handle this
// special case, because it's very common for checkboxes and radio buttons.
if (focusEventTarget && this._isLastInteractionFromInputLabel(focusEventTarget)) {
return 'mouse';
}

return 'program';
}

/**
Expand Down Expand Up @@ -552,6 +567,40 @@ export class FocusMonitor implements OnDestroy {

return results;
}

/**
* Returns whether an interaction is likely to have come from the user clicking the `label` of
* an `input` or `textarea` in order to focus it.
* @param focusEventTarget Target currently receiving focus.
*/
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A much simpler alternative I was considering was to listen to click events inside the InputModalityDetector instead and set the modality to mouse again. I decided against it, because it would've meant that modalityDetected would emit two consecutive mouse interactions on any click which can be annoying for consumers. Also this seems very much like a focus issue so it makes more sense to handle it in the FocusMonitor than the InputModalityDetector.

private _isLastInteractionFromInputLabel(focusEventTarget: HTMLElement): boolean {
const {_mostRecentTarget: mostRecentTarget, mostRecentModality} = this._inputModalityDetector;

// If the last interaction used the mouse on an element contained by one of the labels
// of an `input`/`textarea` that is currently focused, it is very likely that the
// user redirected focus using the label.
if (
mostRecentModality !== 'mouse' ||
!mostRecentTarget ||
mostRecentTarget === focusEventTarget ||
(focusEventTarget.nodeName !== 'INPUT' && focusEventTarget.nodeName !== 'TEXTAREA') ||
(focusEventTarget as HTMLInputElement | HTMLTextAreaElement).disabled
) {
return false;
}

const labels = (focusEventTarget as HTMLInputElement | HTMLTextAreaElement).labels;

if (labels) {
for (let i = 0; i < labels.length; i++) {
if (labels[i].contains(mostRecentTarget)) {
return true;
}
}
}

return false;
}
}

/**
Expand Down