Skip to content
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

retest tabindex change on focusout #32

Open
pkra opened this issue Mar 24, 2021 · 4 comments
Open

retest tabindex change on focusout #32

pkra opened this issue Mar 24, 2021 · 4 comments
Assignees
Labels
documentation Improvements or additions to documentation question Further information is requested

Comments

@pkra
Copy link
Member

pkra commented Mar 24, 2021

Looking at the APG patterns, they leave the tabindex as is when focus leaves the tree. Let's try that as it might help improve UX for revisiting a tree after exploration (cf. https://gitlab.gnome.org/GNOME/orca/-/issues/195#note_1065628).

@pkra pkra added the bug Something isn't working label Mar 24, 2021
@pkra pkra self-assigned this Mar 24, 2021
@pkra
Copy link
Member Author

pkra commented Apr 2, 2021

It doesn't seem to change Orca's behavior; I don't recall how other ATs did - needs more testing.

@pkra
Copy link
Member Author

pkra commented May 28, 2021

After more testing, this doesn't seem to help much. Like Orca, NVDA is fine either way (Chrome, FF). JAWS is ok in Chrome but in Firefox, JAWS browse mode visits both the tree's label and active (tabindex=0) treeitem's label; sometimes visits the treeitem twice.

VoiceOver on Mac is a hot mess, I can't remember what it was like the last time but I feel it's about the same; both Safari and Chrome struggle in general and don't improve with the change.

@pkra
Copy link
Member Author

pkra commented May 28, 2021

We need clearer test instructions and documentation of results.

@pkra
Copy link
Member Author

pkra commented May 28, 2021

For the record, here's the diff of the rough implementation I tested:

diff --git a/lib/navigator.js b/lib/navigator.js
index 21472a0..f61f896 100644
--- a/lib/navigator.js
+++ b/lib/navigator.js
@@ -15,12 +15,10 @@ class navigator {
     this.tree = extractAbstractTree(node);
     this.node.addEventListener('keydown', this.move.bind(this));
     this.node.addEventListener('focusin', () => {
-      this.node.setAttribute('tabindex', '-1');
       this.highlight(true);
     });
     this.node.addEventListener('focusout', () => {
-      this.highlight(false);
-      this.node.setAttribute('tabindex', '0');
+      this.highlight(false, true);
     });
   }
 
@@ -69,7 +67,7 @@ class navigator {
     node.getAttribute('data-owns').split(' ').forEach(id => this.highlightSubtree(boolean, this.node.querySelector(`[data-owns-id="${id}"]`)));
   }
 
-  highlight(boolean) {
+  highlight(boolean, focusOut) {
     const activedescendant =
       this.active().name === this.node.getAttribute('data-owns-id')
         ? this.node
@@ -81,7 +79,7 @@ class navigator {
       activedescendant.focus();
     }
     if (boolean === false) {
-      activedescendant.setAttribute('tabindex', '-1');
+      if (!focusOut) activedescendant.setAttribute('tabindex', '-1');
       activedescendant.classList.remove('is-activedescendant');
     }
   }

@pkra pkra added question Further information is requested documentation Improvements or additions to documentation and removed bug Something isn't working labels May 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation question Further information is requested
Projects
None yet
Development

No branches or pull requests

1 participant