fix: allow tabbing out of [tabindex=-1] elements#24
Conversation
|
@geebru thanks for this. I believe this was my issue with a11y - if the activeElement isn't tabbable (but is focusable), then you can't get the next element in the tabSequence since the current activeElement isn't in there. I also looked at a11y's However looking at the jquery plugin it should be able to query from any element and give us the next tabbable. |
|
@bkucera That makes sense (and good to know the issue I was hitting wasn't just me). I just find it surprising that ally wouldn't have a solution for this since their examples around tabbable and focusable all work as one might expect, so it seems odd that a combination would fail in this way. I should have some more time next week to look into this more - might take one more stab at ally but then see if I can't get the jquery.tabbable plugin working. |
|
@bkucera Secondary note - while perusing ally.js issues and backlog things it looks like they have open collaboration with jQuery UI to actually replace JQUI's own focusable/tabbable with ally's versions if I'm reading this right: medialize/ally.js#41 Seems the biggest blocker to that right now is browser support (JQUI down to IE8, ally at IE10) but otherwise the teams seem amicable to the merger. |
|
I've integrated this fix into #65 |
Long-awaited Cypress 10+ Support for cy.tab() - remove Ally.js dependency (abandoned) - closes #59 - closes #37 - fix focus randomly being lost due (due to ally.js) - closes #46 - upgrade to Cypress 10+, with new compatible type definitions - closes #6 - closes #40 - fix tabbing from focusable elements - fixes #18 - subsumes #24 - follow subject changes in Cypress command chain / events sent in proper order before focus change - closes #47 - fix hanging tests when used with cy.clock - closes #42
Description
Currently the plugin will error if you focus a focusable,
tabindex=-1element and try to tab out of it. This is incorrect as long as the element is capable of being focused programmatically, which -1 is.Solutions:
@bkucera suggested moving from ally.js to tabbable.js but ally.js does include a differentiation query between focusable and tabbable. The changes here allow tabbing out of focusable elements (but will still error if trying to tab from non-focusable elements).
WIP:
Focusing on a -1 element and pressing tab currently goes backwards. I believe this has to do with how the index traversal was setup based on isTabbable and needs to be updated to work based on the next item in the isTabbable tree based on the current isFocusable item.
I'll keep trying things but JS isn't my top language by any means so I definitely welcome help from anyone.
This PR includes a separate HTML in fixtures with a simplified setup, as well a series of rudimentary tests on the fixture. Note that it also adds a skip to the plugins original spec files to speed up the testing flow for this issue and it should be reverted prior to merging (potentially in addition to removing the new HTML/spec files).