-
Notifications
You must be signed in to change notification settings - Fork 250
Cycle focus between the body and page tab sequence #368
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
Cycle focus between the body and page tab sequence #368
Conversation
Codecov Report
@@ Coverage Diff @@
## master #368 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 12 12
Lines 376 381 +5
Branches 109 111 +2
=========================================
+ Hits 376 381 +5
Continue to review full report at Codecov.
|
It's consistent with JSDOM semantics, so it should be fine. Not sure if this would break if someone wasn't using JSDOM, but that's pretty unlikely and we could always patch this later if it becomes an issue.
Not sure I understand the context here, is it only an issue in some versions of JSDOM? |
I would guess so. But I also haven't investigated much (besides commenting out relevant pieces of code). I found out because I was trying to understand what was happening in the |
I'm not sure, it's your call if you think it needs to be fixed. If it's definitely just about the JSDOM version I wouldn't worry about it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's merge this, then we can address other stuff in another pill request.
@all-contributors please add @juanca for code and tests |
I've put up a pull request to add @juanca! 🎉 |
Thanks so much for your help! I've added you as a collaborator on the project. Please make sure that you review the |
🎉 This PR is included in version 12.0.3 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
Resolves #365
What: Add
document.body
to the cycled elements in tab sequenceWhy: The browser allows the active element to be
document.body
when the next selected element is outside the page tab sequence. When the active element is the body (e.g. initial state) then the next reasonable selected element is the first element in the page tab sequence.How:
document.body
accordinglyChecklist:
I was assuming that invoking
document.body.focus
would be undesirable -- opting for justpreviousElement.blur
as a means of setting active element to thebody
. However, it seems thatdocument.body.focus
is working well enough. Do we want to add this kind of logic?All tests pass when not mutating
tabindex
fortab
. Do we want to clean that up in this PR? Or is there some version of jsdom this is intended to support?