-
Notifications
You must be signed in to change notification settings - Fork 1
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
[n/a] add Tabs documentation #3
Conversation
data-action="keyup->tabs#cycleTabs" | ||
role="tablist" | ||
> | ||
{['Tab 1', 'Tab 2', 'Tab 3'].map((text, i) => ( |
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.
Super minor, but the JSX here might potentially trip up newer folks who aren't familiar what parts are Stimulus syntax and what's JSX.
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.
fair, i struggled with that. should we commit to twig demo code? the hold up for me was i couldnt actually (or easily) verify the twig code the way i could the jsx code
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.
oh and then i thought i could just manually do multiple elements but i figured the loop logic was helpful for the attributes even if it was in jsx 🤷
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.
somewhat related, entirely nit/subjective: did you consider having everything in a single array? like
tabsAndTabpanels = [
{
tabContent: …,
tabpanelContent: …,
},
…
]
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.
This is great. Thoughts about improving accessibility?
- arrow, home, and end keys to switch between tabs
- tabindex="0" on the active tabpanel
- aria-controls / aria-labelledby between tabs and tabpanels
- aria-selected on tabs
- aria-label on the tablist
Refs:
👍 you just mean home & end are missing?
👍
this is already there unless i'm missing something?
👍 |
huh. correct. don't know what I was looking at ha
oh interesting I missed that
"huh. correct. don't know what I was looking at ha" take 2 🤷♂️ |
667bc3f
to
bd4ddf2
Compare
🚀 https://code.viget.com/stimulus-controllers/tabs/