Fix issue with nested <Tabs> components
#473
Conversation
🦋 Changeset detectedLatest commit: 3fbc696 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 |
✅ Deploy Preview for astro-starlight ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
There was a problem hiding this comment.
Thanks for the quick fix @HiDeoo! First time I’ve seen :scope — super handy.
I’ll admit seeing your demo gives me UI anxiety — not sure anyone should be using nested tabs 😅 — but if people want to I guess we enable them 😆
One quick question: did you test the keyboard navigation by any chance to make sure it also works as expected with this set-up?
| @@ -77,7 +77,7 @@ const { html, panels } = processPanels(panelHtml); | |||
| super(); | |||
| const tablist = this.querySelector<HTMLUListElement>('[role="tablist"]')!; | |||
| this.tabs = [...tablist.querySelectorAll<HTMLAnchorElement>('[role="tab"]')]; | |||
There was a problem hiding this comment.
Don’t we need a similar approach for this selector too?
There was a problem hiding this comment.
We actually do not (which is quite neat as those are not direct children and would need longer selectors):
- The first one is using
querySelector()and notquerySelectorAll()so it only return the first result which is what we want. - The second one is based on the result of the first one so it's also what we want.
Basically only the last one was an issue as it was based on the component (this) and was using querySelectorAll().
Ah yeah definitely not the most common pseudo-class ^^
I lost myself in the tabs making that video 😆
I actually did and forgot to mention it. Here is a new video of only keyboard navigation which work as expected: keyboard.mp4 |
What kind of changes does this PR include?
Description
This pull request fixes an issue reported on Discord regarding nested tabs.
The panels
hiddenproperty was not being set correctly due to the query selector matching all nested panels instead of just the direct children.I updated it to use the :scope CSS pseudo-class to only match the direct children.
I don't think we have a proper way to test this yet so here is a video of a manual test:
demo.mp4