[BREAKING][#666] Default full to attached nodes only; add legacy-full #764
Conversation
🦋 Changeset detectedLatest commit: 5ff50fd The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
Fixes #666 Tabbable has long had legacy behavior that treated detached nodes as visible. This commit makes the default/full `displayCheck` option now behave correctly in that it treates detached nodes as hidden. But since many people may depend on the buggy legacy behavior, a new 'legacy-full' option is added which preserves the legacy behavior, for now.
stefcameron
force-pushed
the
detached-nodes
branch
from
bacfd33 to
5ff50fd
Compare
Codecov Report
@@ Coverage Diff @@
## master #764 +/- ##
==========================================
+ Coverage 97.72% 97.83% +0.10%
==========================================
Files 1 1
Lines 220 231 +11
Branches 106 111 +5
==========================================
+ Hits 215 226 +11
Misses 5 5
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
|
@craigkovatch @idoros I'm fixing #666 which we had logged a while back after this happened: #664 I'm going to publish a major because I'm dropping all IE support now that IE has been sunset for good, so this is the opportunity to fix the default "full" display check behavior. I've added a Sounds/looks good? |
|
Sounds good thank you
…-Craig (mobile)
________________________________
From: Stefan Cameron ***@***.***>
Sent: Wednesday, August 10, 2022 10:05:01 PM
To: focus-trap/tabbable ***@***.***>
Cc: Craig Kovatch ***@***.***>; Mention ***@***.***>
Subject: Re: [focus-trap/tabbable] [BREAKING][#666] Default full to attached nodes only; add legacy-full (PR #764)
@craigkovatch<https://github.com/craigkovatch> @idoros<https://github.com/idoros> I'm fixing #666<#666> which we had logged a while back after this happened: #664<#664>
I'm going to publish a major because I'm dropping all IE support now that IE has been sunset for good, so this is the opportunity to fix the default "full" display check behavior.
I've added a legacy-full option that still does the old behavior which folks can use if they need an escape valve for this new major. But going forward, the intention is to eventually drop the legacy behavior completely.
Sounds/looks good?
—
Reply to this email directly, view it on GitHub<#764 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/ACTWJEIOUAHMIWRT2TGG3N3VYQDO3ANCNFSM56FQ7JVQ>.
You are receiving this because you were mentioned.Message ID: ***@***.***>
|
|
Thanks for checking it, both of you! Merging. |
| @@ -135,7 +135,7 @@ These options apply to all APIs. | |||
|
|
|||
| ### displayCheck option | |||
|
|
|||
| Type: `full` | `non-zero-area` | `none` . Default: `full`. | |||
| Type: `full` | `full-with-hidden` | `non-zero-area` | `none` . Default: `full`. | |||
There was a problem hiding this comment.
I thought the new name was legacy-full?
There was a problem hiding this comment.
Yes, it's legacy-full. Thanks for catching that. Will fix it right away.
Fixes #666
Tabbable has long had legacy behavior that treated detached nodes as
visible. This commit makes the default/full
displayCheckoptionnow behave correctly in that it treates detached nodes as hidden.
But since many people may depend on the buggy legacy behavior, a
new 'legacy-full' option is added which preserves the legacy
behavior, for now.
PR Checklist
Please leave this checklist in your PR.
typeof document/window !== 'undefined'before using it in code that gets executed on load.yarn changesetlocally to add one, and follow the prompts).