fix: tabbable sometimes incorrectly includes children of a web component

[benjamin-t-frost]

Use Case

A browser will skip tab-targeting an element if it is the only child of a web component which has a tabindex of -1.

Example

<my-input tabindex="-1">
  <input id="input" type="text" />
</my-nput>

Expected behaviour

tabbable() should return an empty list for the example above

Actual behaviour

tabbable() returns a list which includes the input depicted above

Impact

This bug can cause a focus-trap to behave erratically when nearing the end of focusable elements.

PR Checklist

Please leave this checklist in your PR.

• Source changes maintain stated browser compatibility.
• Issue being fixed is referenced.
• Unit test coverage added/updated.
• E2E test coverage added/updated.
• Typings added/updated.
• Changes do not break SSR:
  • Careful to test typeof document/window !== 'undefined' before using it in code that gets executed on load.
• README updated (API changes, instructions, etc.).
• Changes to dependencies explained.
• Changeset added (run yarn changeset locally to add one, and follow the prompts).
  • EXCEPTION: A Changeset is not required if the change does not affect any of the source files that produce the package bundle. For example, demo changes, tooling changes, test updates, or a new dev-only dependency to run tests more efficiently should not have a Changeset since it will not affect package consumers.

[changeset-bot]

🦋 Changeset detected

Latest commit: ec56737

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
tabbable	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[stefcameron]

👀

[codecov]

Codecov Report

Merging #681 (ec56737) into master (d40f247) will increase coverage by 0.06%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #681      +/-   ##
==========================================
+ Coverage   97.66%   97.72%   +0.06%     
==========================================
  Files           1        1              
  Lines         214      220       +6     
  Branches      104      106       +2     
==========================================
+ Hits          209      215       +6     
  Misses          5        5              

Impacted Files	Coverage Δ
src/index.js	97.72% <100.00%> (+0.06%)	⬆️

---

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 d40f247...ec56737. Read the comment docs.

[stefcameron]

A very proactive PR, ticking all the boxes. 😄 Thank you! I do have a couple of requests/concerns, though.

[stefcameron]

Thanks for the updates, and for tweaking the comment in the code about what is and is not tabbable to be more generic so there's no confusion in the future about shadows with 1 vs multiple children.

Looks good!

[benjamin-t-frost]

@stefcameron No problem 🙇

How long do you think until this gets released and updated in focus-trap? Not rushing just need to update my ticket ~

[stefcameron]

How long do you think until this gets released and updated in focus-trap? Need to update my ticket ~

I'll release tabbable as soon as I merge this, and then will do a release of focus-trap. So you should see a new version of focus-trap out later today or tomorrow.

[stefcameron]

Getting published to [email protected] now.

[stefcameron]

[email protected] is getting published now. :)