-
Notifications
You must be signed in to change notification settings - Fork 86
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
fix(tab-nav): content clickable for screen readers #1692
fix(tab-nav): content clickable for screen readers #1692
Conversation
✅ Deploy Preview for marvelous-moxie-a6e2fe ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
|
||
@State() hasFocus: boolean = false; | ||
|
||
@Event({ eventName: 'scale-select' }) scaleSelect: EventEmitter; |
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.
should we scope this event to avoid collisions?
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.
we could, but I think it makes sense to also have this event be "public", no?
handleSelect(event) { | ||
const nextTab = event.target as HTMLScaleTabHeaderElement; | ||
// Act only if it's a direct child | ||
if (this.getAllEnabledTabs().includes(nextTab)) { |
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.
...or this takes care of collisions?
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 takes care of collisions with nested tabs… (an issue we had reported not long ago)
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.
actually with nested anything haha
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.
LGTM, minor questions kinda only answered themselves
Fixes #957
I removed the
click
handler fromtab-nav.tsx
and made thescale-tab-header
emit ascale-select
event.@nowseemee let me know what you think!