Add `tab` helper (to simulate "tabbing" through focusable elements) #1113

NullVoxPopuli · 2021-08-24T16:28:53Z

Continuation of: https://github.com/emberjs/ember-test-helpers/pull/771/files

Tested Locally:

Firefox v91.0.2 on Ubuntu Linux

Turbo87 · 2021-09-07T22:10:19Z

does this respect the fact that on macOS Chrome only links are tab enabled by default, and links won't receive focus?

NullVoxPopuli · 2021-09-07T22:13:46Z

@Turbo87 I am not sure what you mean? can you describe that behavior? and/or how it's different from other browsers?

Today, I only tested the test suite in Firefox on linux.
If Mac has different behaviors, would you be open to a separate PR that expands the test matrix to include MacOS?

Turbo87 · 2021-09-08T17:10:02Z

I am not sure what you mean? can you describe that behavior?

on macOS Chrome links are not part of the tab order. in other words: when you press Tab it won't focus links.

and/or how it's different from other browsers?

Firefox on macOS does focus links. Chrome on other platforms AFAIK also focusses links.

NullVoxPopuli · 2021-09-08T21:30:48Z

I can focus links via tab (on Mac, Chrome)

but anyway, running the tests in Chrome on Mac still results in a successful test suite.

Turbo87 · 2021-09-08T22:04:15Z

OMG they finally fixed it 😂

forget everything I said... 🙈

MelSumner · 2021-09-14T17:53:14Z

@rwjblue I love this, can we have this please? ^_^

MelSumner

I don't think I can officially approve this but 100% I think it's great!

rwjblue · 2021-09-14T20:30:29Z

I'll try to review in detail this week.

@NullVoxPopuli - Can you use the Co-authored-by: commit header to add attribution for @mogstad's hard work over in #771?

NullVoxPopuli · 2021-09-14T20:42:02Z

sure thing! did I do it right? I've never done a Co-authored by thing

mogstad · 2021-09-15T08:35:50Z

Thanks for picking this up NullVoxPopuli, and bringing it over the finish line.

Re Co-authored I usually just includes it at the bottom of the commit body and not in the title.

does this respect the fact that on macOS Chrome only links are tab enabled by default, and links won't receive focus?

This is the default behaviour in Safari on macOS, and there is a setting to change it.. I haven’t found a way of detecting this setting. The code doesn’t try to accommodate for Safari’s behaviour. Detecting Safari and filter out links ~~with a default tabindex~~ should be rather straightforward.

NullVoxPopuli · 2021-10-06T17:48:11Z

Ok, I've fixed the attribution (thanks again, @mogstad !!!)
And I've rebased.

Lemme know if anything needs to be done for this batch of work :)

Co-Authored-By: Bjarne Mogstad <me@mogstad.co>

rwjblue · 2021-10-08T16:17:36Z

addon-test-support/@ember/test-helpers/dom/-create-tree-walker.ts

+  if (workaroundForIE11) {
+    let acceptNode = filter ? (filter.acceptNode as any as NodeFilter) : null;
+    return ownerDocument.createTreeWalker(
+      root,
+      whatToShow,
+      acceptNode,
+      entityReferenceExpansion
+    );
+  } else {


I think we can probably just say that we don't support new features in IE11 mode. What do you think?

does that require a breaking release of test-helpers?
as long as I can still use this pre-ember 4, I think this sounds great :D

rwjblue · 2021-10-08T16:17:53Z

addon-test-support/@ember/test-helpers/dom/-is-focusable.ts

@@ -1,7 +1,7 @@
 import isFormControl from './-is-form-control';
 import { isDocument, isContentEditable, isWindow } from './-target';

-const FOCUSABLE_TAGS = ['A'];
+const FOCUSABLE_TAGS = ['A', 'SUMMARY'];


Link to spec suggesting that this is focusable?

good call, I should probably drop a bunch of spec links in here

https://html.spec.whatwg.org/multipage/interaction.html#the-tabindex-attribute you may have to ctrl+f to get to it, but this is the nearest anchor/header

addon-test-support/@ember/test-helpers/dom/fire-event.ts

addon-test-support/@ember/test-helpers/dom/tab.ts

rwjblue · 2021-10-08T16:23:17Z

addon-test-support/@ember/test-helpers/dom/tab.ts

+function isVisible(element: Element): boolean {
+  let styles = window.getComputedStyle(element);
+  return styles.display !== 'none' && styles.visibility !== 'hidden';
+}


Hmm, this seems incomplete to me. 🤔

It could also be positioned outside the viewport, right?

yeah -- isVisible kinda has multiple meanings. 🤔

as implemented, it's like the real behavior where the tabbable element can be tabbed to even when it's not in the viewport (and normally this scrolls to the element so that it is in the viewport) -- maybe isVisible should be renamed to isVisibleInDOM and a separate isVisibleInViewport should be implemented just to differentiate, even if unused initially?

addon-test-support/@ember/test-helpers/dom/tab.ts

rwjblue · 2021-10-08T16:32:03Z

tests/helpers/events.js

+            // needs to be get so prop-paths can be used, such as "target.id"
+            step += ` ${get(e, prop)}`;


Can we just look for the specfic ones we care about? I'd rather not use Ember.get here...

thoughts on lodash.get?

@NullVoxPopuli what's the decision here?

gonna leave ember.get, because of the need for nested property path accesses in the tests

tests/unit/dom/tab-test.js

Rename buildKeyboardEvent to _buildKeyboardEvent Rename SUPPORT_INHERT to SUPPORT_INERT

MelSumner

Since the concerns seem addressed and the tests are passing, I'm approving this PR so that we can move things forward. Thank you for working on this!

rwjblue · 2021-10-25T15:20:43Z

Looks like this missed adding registerHooks support. @NullVoxPopuli - would you be able to do that?

NullVoxPopuli · 2021-10-25T15:37:17Z

@rwjblue sure thing -- but for clarification, is registerHooks this: #1142 ?

scalvert · 2021-10-25T16:12:22Z

It is not. It's #878

NullVoxPopuli force-pushed the tab-support branch from 1862730 to 250d6e4 Compare September 7, 2021 21:49

NullVoxPopuli marked this pull request as ready for review September 7, 2021 22:03

MelSumner requested review from Turbo87, krisselden and rwjblue September 7, 2021 22:06

MelSumner reviewed Sep 14, 2021

View reviewed changes

rwjblue requested a review from scalvert September 14, 2021 20:28

NullVoxPopuli force-pushed the tab-support branch from f38524a to ed28046 Compare September 14, 2021 20:41

NullVoxPopuli force-pushed the tab-support branch from ed28046 to 100d90e Compare October 6, 2021 17:47

NullVoxPopuli and others added 2 commits October 6, 2021 15:53

Tab Support -- continuation of emberjs#771

f7e076c

Co-Authored-By: Bjarne Mogstad <me@mogstad.co>

Update Docs

6d25288

NullVoxPopuli force-pushed the tab-support branch from 100d90e to 6d25288 Compare October 6, 2021 19:53

NullVoxPopuli mentioned this pull request Oct 8, 2021

A tab testing once https://github.com/emberjs/ember-test-helpers/pull/1113 is merged CrowdStrike/ember-aria-utilities#4

Open

rwjblue requested changes Oct 8, 2021

View reviewed changes

NullVoxPopuli added 4 commits October 8, 2021 21:23

Remove tree-walker file, it existed only for IE11 support.

6e9362c

Rename buildKeyboardEvent to _buildKeyboardEvent Rename SUPPORT_INHERT to SUPPORT_INERT

Remove more IE11 things

b7113c2

Add reference to focusable tags

197a09d

Tests pass

177686a

NullVoxPopuli requested a review from rwjblue October 10, 2021 18:02

NullVoxPopuli mentioned this pull request Oct 21, 2021

[Maintenance] Extract test setup/teardown helper for dom-using helpers #1142

Open

MelSumner approved these changes Oct 21, 2021

View reviewed changes

MelSumner merged commit 205dd8e into emberjs:master Oct 21, 2021

NullVoxPopuli mentioned this pull request Oct 21, 2021

Tab #771

Closed

rwjblue changed the title ~~Tab support~~ Add tab helper (to simulate "tabbing" through focusable elements) Oct 22, 2021

rwjblue added the enhancement label Oct 22, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add `tab` helper (to simulate "tabbing" through focusable elements) #1113

Add `tab` helper (to simulate "tabbing" through focusable elements) #1113

NullVoxPopuli commented Aug 24, 2021 •

edited

Turbo87 commented Sep 7, 2021

NullVoxPopuli commented Sep 7, 2021

Turbo87 commented Sep 8, 2021

NullVoxPopuli commented Sep 8, 2021 •

edited

Turbo87 commented Sep 8, 2021

MelSumner commented Sep 14, 2021

MelSumner left a comment

rwjblue commented Sep 14, 2021

NullVoxPopuli commented Sep 14, 2021

mogstad commented Sep 15, 2021 •

edited

NullVoxPopuli commented Oct 6, 2021

rwjblue Oct 8, 2021

NullVoxPopuli Oct 8, 2021

rwjblue Oct 8, 2021

NullVoxPopuli Oct 8, 2021

NullVoxPopuli Oct 9, 2021

rwjblue Oct 8, 2021

NullVoxPopuli Oct 9, 2021

rwjblue Oct 8, 2021

NullVoxPopuli Oct 9, 2021

MelSumner Oct 21, 2021

NullVoxPopuli Oct 21, 2021

MelSumner left a comment

rwjblue commented Oct 25, 2021

NullVoxPopuli commented Oct 25, 2021

scalvert commented Oct 25, 2021

		// needs to be get so prop-paths can be used, such as "target.id"
		step += ` ${get(e, prop)}`;

Add tab helper (to simulate "tabbing" through focusable elements) #1113

Add tab helper (to simulate "tabbing" through focusable elements) #1113

Conversation

NullVoxPopuli commented Aug 24, 2021 • edited

Turbo87 commented Sep 7, 2021

NullVoxPopuli commented Sep 7, 2021

Turbo87 commented Sep 8, 2021

NullVoxPopuli commented Sep 8, 2021 • edited

Turbo87 commented Sep 8, 2021

MelSumner commented Sep 14, 2021

MelSumner left a comment

Choose a reason for hiding this comment

rwjblue commented Sep 14, 2021

NullVoxPopuli commented Sep 14, 2021

mogstad commented Sep 15, 2021 • edited

NullVoxPopuli commented Oct 6, 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MelSumner left a comment

Choose a reason for hiding this comment

rwjblue commented Oct 25, 2021

NullVoxPopuli commented Oct 25, 2021

scalvert commented Oct 25, 2021

Add `tab` helper (to simulate "tabbing" through focusable elements) #1113

Add `tab` helper (to simulate "tabbing" through focusable elements) #1113

NullVoxPopuli commented Aug 24, 2021 •

edited

NullVoxPopuli commented Sep 8, 2021 •

edited

mogstad commented Sep 15, 2021 •

edited