feat(cdk/a11y): Add a new InputModalityDetector service to detect the user's current input modality. #22371

zelliott · 2021-03-30T17:54:35Z

Note: This is a first-stab at an input modality detection service and I expect there to be some churn regarding its exact behavior, so feel free to leave many comments.

Motivation

In our app, we conditionally render strong focus styles on elements if the element is determined to have been focused via keyboard. This is done by attaching a global FocusMonitor to the document.body. One problem with this approach is that we can only detect input modality changes when focus also changes (see #21555 for why this is a problem with aria-activedescendant elements). The purpose of the InputModalityDetector service is to allow users to detect a broader array of input modality changes, regardless of whether focus changes.

Approach

The InputModalityDetector attempts to detect input modality by listening to keydown, mousedown, and touchstart events, and attributing each event to its corresponding input modality ('keyboard', 'mouse', 'touch'). For example, when a keydown event is fired, the service detects the 'keyboard' input modality. I chose this small subset of events as I thought they best represented legitimate, intentional, and committed user interaction.

The service uses some additional heuristics under-the-hood to support scenarios when these events are fired but likely don't represent an actual change in input modality. For example:

By default, the service ignores modifier keys (please see the comment in the source for why). We allow the user to override this behavior if they wish, but would cause some difference in behavior between VoiceOver and other SRs.
The service ignores mousedown events that occur too closely after a touchstart event (again, please see the comments in the source for why).

Demo

https://stackblitz.com/edit/angular-ivy-9cxqav?file=src/app/input-modality-detector.ts

Behavior

The service essentially works as you would expect when interacting with a page. Interesting edge cases are as follows:

The input modality is null when the service is first instantiated, as we have no clue what the user's input modality is.
Interacting with an inert element (e.g. clicking empty space or just pressing a random key) changes the input modality accordingly. My current opinion is that this is fine, as it represents an attempted interaction with the page.
If a user's input modality changes in the middle of interacting with a built-in popup control such as a <select> or <input type="datepicker">, this change is not detected. This is because no events are emitted.
If a user clicks while pressing a modifier key such as Control or Option (to either "right-click" or "open in a new tab"), this is detected as mouse input modality (as modifier keys are ignored by default, as mentioned earlier).
If a user is using a screen reader, input modality for the most part will not be detected (there's a comment in the source that goes into more detail).

Testing

I'm planning on testing the service on the following platforms, browsers, and screen readers:

Platforms: Web (Windows, Mac, Chrome OS), Mobile (Android, iOS)
Browsers: Chrome, FF, Safari, Edge, IE11.
SRs: NVDA, JAWS, VoiceOver, ChromeVox, Android Accessibility Suite.

I've currently tested most of the combinations above, with the exception of Chrome OS (+ChromeVox), Android (+Android Accessibility Suite), and IE11.

Prior Art / Resources

zelliott · 2021-03-30T17:57:27Z

src/cdk/a11y/input-modality/input-modality-detector.ts

+ * The amount of time needed to pass after a touchstart event in order for a subsequent mousedown
+ * event to be attributed as mouse and not touch.
+ */
+export const TIME_AFTER_TOUCH_MS = 650;


This was grabbed from https://github.com/angular/components/blob/master/src/cdk/a11y/focus-monitor/focus-monitor.ts#L34 and from what I can tell they're both used for the same purpose? Should one of these usages import the other or should we move it into some shared file?

Yeah, I think focus monitor should probably consume this. We should also preserve the comment about the origin of this value. However, we probably want to avoid adding this symbol to the public API.

src/cdk/a11y/input-modality/input-modality-detector.ts

zelliott · 2021-03-30T18:06:03Z

src/cdk/a11y/input-modality/input-modality-detector.ts

+  private _onKeydown = (event: KeyboardEvent) => {
+    // If this is one of the keys we should ignore, then ignore it and don't update the input
+    // modality to keyboard.
+    if (this._options?.ignoreKeys?.some(key => key === event.key)) { return; }


I'd prefer to use event.key instead of keyCode, but if you're still targeting IE11 it looks like it has only partial support (https://caniuse.com/?search=KeyboardEvent.key), which may not be good enough (I'll need to test).

Yeah, we unfortunately still use keyCode everywhere and plan to switch to key or code once IE11 support is dropped (see #7374). We plan to deprecate IE11 support in v12 (this May) and fully remove it in either v13 or v14 based on community feedback.

Gotcha. Switched to keyCode.

jelbourn

Overall this looks good; if we can show that FocusMonitor can use this it would be good to go IMO

css @mmalerba and @crisbeto who have done most of the work on FocusMonitor

jelbourn · 2021-03-31T20:46:56Z

src/cdk/a11y/input-modality/input-modality-detector.ts

+ * The amount of time needed to pass after a touchstart event in order for a subsequent mousedown
+ * event to be attributed as mouse and not touch.
+ */
+export const TIME_AFTER_TOUCH_MS = 650;


Yeah, I think focus monitor should probably consume this. We should also preserve the comment about the origin of this value. However, we probably want to avoid adding this symbol to the public API.

jelbourn · 2021-03-31T20:47:47Z

src/cdk/a11y/input-modality/input-modality-detector.ts

+ * Event listener options that enable capturing and also mark the listener as passive if the browser
+ * supports it.
+ */
+const opts = normalizePassiveListenerOptions({


Suggested change

const opts = normalizePassiveListenerOptions({

const modalityEventListenerOptions = normalizePassiveListenerOptions({

jelbourn · 2021-03-31T20:50:51Z

src/cdk/a11y/input-modality/input-modality-detector.ts

+  get inputModality(): InputModality {
+    return this._inputModality;
+  }


Any reason to make this a getter vs. making the actual property public?

Only to prevent people from setting it themselves for whatever reason. Happy to change it to a public property, just LMK.

Note that now the getter is more necessary because we're grabbing the value from the underlying BehaviorSubject.

jelbourn · 2021-03-31T20:51:22Z

src/cdk/a11y/input-modality/input-modality-detector.ts

+  /** The underlying ReplaySubject that emits input modality changes. */
+  private readonly _change = new ReplaySubject<InputModality>(1);
+
+  /** The current / last detected input modality by this service. */


Suggested change

/** The current / last detected input modality by this service. */

/** The most recently detected input modality by this service. */

jelbourn · 2021-03-31T20:53:09Z

src/cdk/a11y/input-modality/input-modality-detector.ts

+  private _onKeydown = (event: KeyboardEvent) => {
+    // If this is one of the keys we should ignore, then ignore it and don't update the input
+    // modality to keyboard.
+    if (this._options?.ignoreKeys?.some(key => key === event.key)) { return; }


Yeah, we unfortunately still use keyCode everywhere and plan to switch to key or code once IE11 support is dropped (see #7374). We plan to deprecate IE11 support in v12 (this May) and fully remove it in either v13 or v14 based on community feedback.

crisbeto · 2021-04-01T08:33:01Z

src/cdk/a11y/input-modality/input-modality-detector.ts

+ *    these keys so as to not update the input modality.
+ */
+export const INPUT_MODALITY_DETECTOR_DEFAULT_OPTIONS: InputModalityDetectorOptions = {
+  ignoreKeys: ['Alt', 'Control', 'Meta', 'Shift'],


These are all modifier keys so shouldn't we be looking at KeyboardEvent.altKey etc.? We have some helpers for dealing with modifiers in @angular/cdk/keycodes already.

I considered that, but I wanted to give users the ability to configure which keys they'd like to ignore/not ignore, and it seemed easiest to support that via a single ignoreKeys API.

It could have a modifiers array and ignoreKeys. If I remember correctly, the keydown will dispatch Alt if you just press the Alt key once, but if you pressed Alt + DOWN_ARROW, you'd get a DOWN_ARROW event with the Alt modifier.

From my testing, if I press Alt + Down Arrow, I get both an Alt and a Down Arrow event, so this should work fine (I tested with a number of modifier keys and on both Mac and Windows). Changing the API to both modifiers and ignoreKeys increases its surface error, and it's not clear to me what the benefit is (do we think it'll be clearer to users?).

src/cdk/a11y/input-modality/input-modality-detector.ts

crisbeto · 2021-04-01T08:36:36Z

src/cdk/a11y/input-modality/input-modality-detector.ts

+  }
+
+  /** The underlying ReplaySubject that emits input modality changes. */
+  private readonly _change = new ReplaySubject<InputModality>(1);


You could replace the ReplaySubject with a BehaviorSubject which has identical behavior, but it also allows you to read the latest value which can then be used for the inputModality getter.

This was actually my previous implementation, but I ended switching to a ReplaySubject because I didn't like how the BehaviorSubject emitted null at the start. My thinking was that inputModalityChange should only emit when the input modality changes, and I didn't want subscribers to have to filter out the first null value in their logic. WDYT?

You could add .pipe(skip(1)) when exposing the subject publicly. It's not a bit deal to leave it as it is though.

Okay, I don't feel strongly so I switched to the BehaviorSubject.

crisbeto · 2021-04-01T08:37:08Z

src/cdk/a11y/input-modality/input-modality-detector.ts

+    this.inputModalityChange =  this._change.pipe(distinctUntilChanged());
+
+    // Add the event listeners used to detect the user's input modality.
+    document.addEventListener('keydown', this._onKeydown, opts);


All of these events should be bound outside of the Angular zone.

Thanks, I made this change, but it's causing some problems in the dev-app demo and Stackblitz. If you take a look at the Stackblitz (I updated with the new implementation of binding these event listeners outside of the ngZone), you'll notice that the input modality no longer updates in the view (though of course the subscription still emits). Without knowing too much about zones my it seems like Angular isn't running CD because it doesn't think anything is changing. Is this expected behavior? If so, I'll need to update the dev-app demo accordingly.

If the listener is bound outside of the zone, you'll have to use NgZone.run when updating the view, otherwise change detection won't run. It's a little cumbersome, but we may end up triggering unnecessary change detections otherwise.

Gotcha, done.

src/cdk/a11y/input-modality/input-modality-detector.ts

crisbeto · 2021-04-01T08:42:31Z

src/cdk/a11y/input-modality/input-modality-detector.ts

+
+  constructor(
+    platform: Platform,
+    @Inject(DOCUMENT) document: Document,


AFAIK there are some issues in ViewEngine with typing constructor parameters as Document. The way we work around it in other places is to type the parameter as any and then cast it to Document afterwards.

I did notice this when looking at other services. Do you recall how to reproduce these issues? I'd prefer to be able to reproduce the issue to confirm the any + cast is actually needed, but if you're confident, then I'll just go ahead and make the change.

I think that we were running into it in our Angular Universal integration tests. There's a readme under src\universal-app\DEBUG.md.

I added the InputModalityDetector to the universal kitchen sink app as-is, and ran bazel test //src/universal-app:server_test --test_output=all. The target passed, and I didn't see any relevant console warnings/logs. Is there something in particular I should be looking for?

src/cdk/a11y/input-modality/input-modality-detector.ts

zelliott · 2021-04-01T18:03:04Z

if we can show that FocusMonitor can use this it would be good to go IMO

I think I responded to all open comments, but I still need to evaluate integration with FocusMonitor.

zelliott · 2021-04-06T16:57:14Z

It's possible to update FocusMonitor to use this new InputModalityDetector service to determine focus origin, but it requires us to tweak the API of this service a bit, and causes some changes in FocusMonitor's behavior. db685c0 demonstrates one potential integration. I left comments on the commit above documenting various things and explaining any changes in behavior. I'll let people look over the commit before I add the changes to this PR.

Additionally, here's a Stackblitz demo-ing integration: https://stackblitz.com/edit/angular-ivy-ls9hgx?file=src%2Fapp%2Ffocus-monitor.ts

src/cdk/a11y/input-modality/input-modality-detector.ts

mmalerba

LGTM

@jelbourn do you want to put this straight into master or create a feature branch so that the focus monitor can be migrated to use this before merging to master?

zelliott · 2021-04-07T00:28:59Z

@jelbourn do you want to put this straight into master or create a feature branch so that the focus monitor can be migrated to use this before merging to master?

I'd recommend a feature branch. Please see #22371 (comment) regarding integration with FocusMonitor. I left a number of open comments on the commit linked to in that comment.

jelbourn · 2021-04-07T18:44:40Z

I've updated the PR to target a feature branch called input-modality

src/cdk/a11y/input-modality/input-modality-detector.ts

jelbourn · 2021-04-07T18:47:59Z

src/cdk/a11y/input-modality/input-modality-detector.ts

+/**
+ * Service that detects the user's input modality.
+ *
+ * Note: This service will not update the input modality when a user is navigating with a screen


Suggested change

* Note: This service will not update the input modality when a user is navigating with a screen

* This service does not update the input modality when a user navigates with a screen

nit: slightly more active voice, present tense

jelbourn · 2021-04-07T18:55:19Z

src/cdk/a11y/input-modality/input-modality-detector.ts

+@Injectable({ providedIn: 'root' })
+export class InputModalityDetector implements OnDestroy {
+  /** Emits when the input modality changes. */
+  readonly inputModalityChange: Observable<InputModality>;


Is it redundant to have inputModalityDetector.inputModalityChange? Maybe we should shorten to just
modalityChanges or changes?

I don't feel strongly, changed to modalityChanges to align with mostRecentModality.

jelbourn · 2021-04-07T18:57:56Z

src/cdk/a11y/input-modality/input-modality-detector.ts

+  /** Returns the most recently detected input modality. */
+  get inputModality(): InputModality {
+    return this._inputModality.value;
+  }


Suggested change

/** Returns the most recently detected input modality. */

get inputModality(): InputModality {

return this._inputModality.value;

}

/** The most recently detected input modality. */

get mostRecentModality(): InputModality {

return this._inputModality.value;

}

What do you think about making this mostRecentModality?

jelbourn · 2021-04-07T19:01:11Z

src/cdk/a11y/input-modality/input-modality-detector.ts

+  constructor(
+      private readonly _platform: Platform,
+      ngZone: NgZone,
+      @Inject(DOCUMENT) document: Document,


Suggested change

@Inject(DOCUMENT) document: Document,

@Inject(DOCUMENT) document: Document|Document,

Using Document here will break in SSR because TypeScript preserves type information on decorated params, and Document doesn't existing in a nodejs environment. We can use this weird workaround to trick TS into not emitting the type as-is.

Can go in a follow-up, but you can test this by adding something to make this API run as part of our kitchen-sink.ts component (which is our SSR test component).

See the thread at #22371 (comment). The kitchen sink e2e test seems to pass - what should I be looking for?

Hmm, maybe something changed in the tooling so that we don't have to worry about this any more ¯\(ツ)/¯

zelliott · 2021-04-07T23:05:57Z

@jelbourn I think I may have accidentally pulled in a commit from annieyw (212c026) when rebasing my branch off of master. Maybe we can address this by updating the remote input-modality branch? My git knowledge is forever out of date. :\ LMK if there's something I should do on my end.

mmalerba · 2021-04-07T23:17:27Z

I synced input-modality up to master, it should go away if you rebase on input-modality

zelliott · 2021-04-07T23:44:45Z

Tried that (and also tried an empty commit to get the rebase picked up), to no avail. It's weird because comparing local with remote here (https://github.com/angular/components/compare/input-modality...zelliott:input-modality?expand=1) shows the commits I'd expect, whereas this PR has an extra one.

zelliott · 2021-04-08T00:13:55Z

Okay fixed by squashing.

src/cdk/a11y/input-modality/input-modality-detector.spec.ts

jelbourn

lgtm - I also fixed the rebase issue

Adds a new InputModalityDetector service to detect a user's input modality. feat(cdk/a11y): Small fixes to InputModalityDetector feat(cdk/a11y): Add new API goldens feat(cdk/a11y): Respond to reviewer feedback feat(cdk/a11y): Respond to additional PR reviewer feedback feat(cdk/a11y): Respond to PR feedback feat(cdk/a11y): Accept new goldens feat(cdk/a11y): Empty commit to try to get a rebase picked up

… user's current input modality. (#22371)

angular-automatic-lock-bot · 2021-05-15T00:03:18Z

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}

google-cla bot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Mar 30, 2021

zelliott commented Mar 30, 2021

View reviewed changes

src/cdk/a11y/input-modality/input-modality-detector.ts Show resolved Hide resolved

zelliott commented Mar 30, 2021

View reviewed changes

zelliott marked this pull request as ready for review March 30, 2021 18:09

zelliott requested review from devversion, jelbourn, mmalerba and a team as code owners March 30, 2021 18:09

zelliott marked this pull request as draft March 30, 2021 18:10

jelbourn reviewed Mar 31, 2021

View reviewed changes

crisbeto reviewed Apr 1, 2021

View reviewed changes

mmalerba reviewed Apr 6, 2021

View reviewed changes

src/cdk/a11y/input-modality/input-modality-detector.ts Show resolved Hide resolved

mmalerba approved these changes Apr 6, 2021

View reviewed changes

jelbourn added target: feature This PR is targeted for a feature branch (outside of main and semver branches) and removed target: minor This PR is targeted for the next minor release labels Apr 7, 2021

jelbourn changed the base branch from master to input-modality April 7, 2021 18:44

jelbourn reviewed Apr 7, 2021

View reviewed changes

zelliott force-pushed the input-modality branch from 5c21d10 to f0ed573 Compare April 7, 2021 22:57

zelliott marked this pull request as ready for review April 7, 2021 23:30

zelliott force-pushed the input-modality branch from 189878c to 427ad81 Compare April 8, 2021 00:12

raymondji reviewed Apr 8, 2021

View reviewed changes

src/cdk/a11y/input-modality/input-modality-detector.spec.ts Outdated Show resolved Hide resolved

jelbourn force-pushed the input-modality branch from 185ebb0 to 3ca1a94 Compare April 12, 2021 17:15

zelliott requested a review from andrewseguin as a code owner April 12, 2021 17:15

jelbourn force-pushed the input-modality branch from 3ca1a94 to b172952 Compare April 12, 2021 17:18

jelbourn approved these changes Apr 12, 2021

View reviewed changes

jelbourn added the action: merge The PR is ready for merge by the caretaker label Apr 12, 2021

mmalerba added the merge safe label Apr 12, 2021

zelliott added 2 commits April 14, 2021 08:40

feat(cdk/a11y): Respond to additional PR feedback.

0117c1b

mmalerba force-pushed the input-modality branch from b172952 to 0117c1b Compare April 14, 2021 15:40

mmalerba merged commit fbf8e44 into angular:input-modality Apr 14, 2021

jelbourn pushed a commit that referenced this pull request Apr 26, 2021

feat(cdk/a11y): Add a new InputModalityDetector service to detect the…

de32327

… user's current input modality. (#22371)

jelbourn pushed a commit that referenced this pull request Apr 26, 2021

feat(cdk/a11y): Add a new InputModalityDetector service to detect the…

046671a

… user's current input modality. (#22371)

angular-automatic-lock-bot bot locked and limited conversation to collaborators May 15, 2021

	const opts = normalizePassiveListenerOptions({
	const modalityEventListenerOptions = normalizePassiveListenerOptions({

	/** The current / last detected input modality by this service. */
	/** The most recently detected input modality by this service. */

	* Note: This service will not update the input modality when a user is navigating with a screen
	* This service does not update the input modality when a user navigates with a screen

	@Inject(DOCUMENT) document: Document,
	@Inject(DOCUMENT) document: Document\|Document,

feat(cdk/a11y): Add a new InputModalityDetector service to detect the user's current input modality. #22371

feat(cdk/a11y): Add a new InputModalityDetector service to detect the user's current input modality. #22371

Conversation

zelliott commented Mar 30, 2021

Motivation

Approach

Demo

Behavior

Testing

Prior Art / Resources

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jelbourn left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zelliott Apr 2, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zelliott commented Apr 1, 2021

zelliott commented Apr 6, 2021 • edited

mmalerba left a comment

Choose a reason for hiding this comment

zelliott commented Apr 7, 2021

jelbourn commented Apr 7, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zelliott commented Apr 7, 2021 • edited

mmalerba commented Apr 7, 2021

zelliott commented Apr 7, 2021

zelliott commented Apr 8, 2021 • edited

jelbourn left a comment

Choose a reason for hiding this comment

angular-automatic-lock-bot bot commented May 15, 2021

zelliott Apr 2, 2021 •

edited

zelliott commented Apr 6, 2021 •

edited

zelliott commented Apr 7, 2021 •

edited

zelliott commented Apr 8, 2021 •

edited