Cycle focus between the body and page tab sequence

[juanca]

Resolves #365

What: Add document.body to the cycled elements in tab sequence

Why: 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:

• Consolidate logic around getting the next element to focus
• If the current active element is on the edges of the page tab sequence then focus the document.body accordingly

Checklist:

• Documentation
• Tests
• Typings
• Ready to be merged
  

1. I was assuming that invoking document.body.focus would be undesirable -- opting for just previousElement.blur as a means of setting active element to the body. However, it seems that document.body.focus is working well enough. Do we want to add this kind of logic?

2. All tests pass when not mutating tabindex for tab. Do we want to clean that up in this PR? Or is there some version of jsdom this is intended to support?

   • Update: handled in refactor(tab): remove tabIndex set and unset for jsdom #379

[codecov]

Codecov Report

Merging #368 into master will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##            master      #368   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           12        12           
  Lines          376       381    +5     
  Branches       109       111    +2     
=========================================
+ Hits           376       381    +5     

Impacted Files	Coverage Δ
src/tab.js	100.00% <100.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f4eae9c...6b13951. Read the comment docs.

[nickmccurdy]

I was assuming that invoking document.body.focus would be undesirable -- opting for just previousElement.blur as a means of setting active element to the body. However, it seems that document.body.focus is working well enough. Do we want to add this kind of logic?

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.

All tests pass when not mutating tabindex for tab. Do we want to clean that up in this PR? Or is there some version of jsdom this is intended to support?

Not sure I understand the context here, is it only an issue in some versions of JSDOM?

[juanca]

All tests pass when not mutating tabindex for tab. Do we want to clean that up in this PR? Or is there some version of jsdom this is intended to support?

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 tab method. Perhaps something to think about later (or not).

[nickmccurdy]

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.

[kentcdodds]

Let's merge this, then we can address other stuff in another pill request.

[kentcdodds]

@all-contributors please add @juanca for code and tests

[allcontributors]

@kentcdodds

I've put up a pull request to add @juanca! 🎉

[kentcdodds]

Thanks so much for your help! I've added you as a collaborator on the project. Please make sure that you review the other/MAINTAINING.md and CONTRIBUTING.md files (specifically the bit about the commit messages and the git hooks) and familiarize yourself with the code of conduct (we're using the contributor covenant). You might also want to watch the repo to be notified when someone files an issue/PR. Please continue to make PRs as you feel the need (you can make your branches directly on the repo rather than your fork if you want). Thanks! And welcome to the team :)

[kentcdodds]

🎉 This PR is included in version 12.0.3 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀