-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Tabs: add "with links" Storybook example #67699
Conversation
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.
To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
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.
I hope the example added here can represent how Tabs
could be integrated with links and a router. There are of course many other ways, and it would largely depend on the routing library (for example, look at ariakit's examples with react router and next.js).
What do folks think?
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.
I'm not worried about people being able to integrate specific routing libraries, as much as I'm worried about people being able to get the accessibility details right for their use case. I think we should at least have a description outlining the accessibility behaviors you need to look for when implementing a given use case:
- When the link doesn't push an item to browser history, but the state can be navigated to using an href (the current example output
<a href="foo" role="tab">
is fine) - When the link does push an item to browser history (it should be
<a href="foo">
and no tab semantics — do we even support this yet?)
I think these are the two main cases but I'm not 100% sure on the accessibility requirements. For example the Next.js example on Ariakit seems lacking to me, as a screen reader user would not be able to perceive that the browser window navigated to a new page (@diegohaz Thoughts?).
We can see examples of the latter case (tab-like styles on what are just semantically nav links):
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.
I agree that the Ariakit examples on Tabs and routing need clearer instructions on how to use them.
In the end, this is about design.
ARIA roles guide users on interacting with elements. Rather than focusing on these roles, designers should first consider how users will engage with a widget and the rest of the page, always aiming for the best user experience. If it's determined that having navigation as a single tab stop navigable with arrow keys improves user experience, with the URL state change being secondary, then you should find the role that best represents this interaction model (which could be tab
), just as you would choose visuals that help sighted users intuitively understand how to interact with the elements the way it's designed.
Design comes first. Then, semantic HTML. If needed, ARIA follows. Everyone involved in design should prioritize accessibility, not because of ARIA, but because it's fundamental to UX. If design fails, everything else is compromised.
That being said, you can't cover all of this in the docs, so it might be better to describe various scenarios that people can refer to for their specific use cases.
When designing websites, you generally want your navigation areas to be lists of semantic, tabbable links, as this is what most users are familiar with. However, if it's a secondary widget, like a sidebar section where changing tabs only affects the URL search params, this approach might differ. In a desktop-like app, such as an online code editor, users generally expect to navigate past the open file list with a single tab and move through them using the arrow keys, as that's how most code editors work. You can choose to update the URL state to represent the selected tab, which could facilitate sharing, but this is a secondary effect. If your target audience consists of VIM users, you'll probably need to design it in a way that's more familiar to them.
Again, design comes first.
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.
When the link doesn't push an item to browser history, but the state can be navigated to using an href (the current example output is fine)
Is an <a />
that doesn't push an item to browser history a good pattern to recommend in general? Why not use a button
in that case?
I'm struggling to understand what we think a good "Tabs with links" example is. Tabs
offers, at a high level, the semantics of tablist/tab/tabpanel, composite-like keyboard navigation for the tablist, and a certain look/feel for its tabs (including the indicator).
For traditional "lists of links", the semantics are list > listitem > link
— does it make any sense to override tabs semantics? What about other attributes like aria-controls
, aria-selected
, etc? It just feels like, semantically, anything that can't work with tabs semantics would not be well-aligned with the component.
In terms of keyboard navigation, links are usually expected to be navigated with the tab key.
The look&feel really feels like the "added value" that the component would offer to a consumer.
In short, I'm struggling to identify what is a good canonical example that we can use for "tabs with links".
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.
In short, I'm struggling to identify what is a good canonical example that we can use for "tabs with links".
Same. That's exactly why I left this comment last week.
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.
I agree that we probably shouldn't support the "links that look like Tabs" pattern. Even though it has been requested before, it's usually bad for user expectations when an identical-looking piece of UI can behave very differently.
The only use case I can see as potentially useful, is this one:
• When the link doesn't push an item to browser history, but the state can be navigated to using an href
In other words, cases when each tabpanel can be accessed with a unique URL, but selecting a tab from the tablist doesn't alter the browser history stack. The benefit of using <a role="tab">
here is that mouse users can command-click to open in a new window, or right-click to copy the URL. And I don't think there are any downsides, since screen reader users won't be confused by the history stack changing unexpectedly when clicking on what is announced as a tab.
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.
I'm not sure if the suggested use-case is "good practice" and frequent enough to warrant a dedicated Storybook example, at least for now. If no one is strongly opposed, I suggest dropping this effort for the time being, and revisiting if needed in the future.
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.
Yes, the use case is a bit contrived 😅 Totally fine with dropping.
Or, since we do get these requests from time to time, it may be worth documenting somewhere that we don't recommend people try using Tabs with links, because of all these reasons.
onSelect={ ( newTabId ) => { | ||
goTo( newTabId ?? undefined ); | ||
} } |
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 feels correct, even if it could be redundant since the SimpleRouterLink
component also calls goTo
internally. What do folks think:?
border-radius: 0; | ||
background: transparent; | ||
border: none; | ||
box-sizing: border-box; |
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 effectively needed to make sure that the tab indicator measures the tab size correctly
const { goTo } = useContext( SimpleRouterContext ); | ||
|
||
return ( | ||
<a |
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.
If we're encouraging folks to use links, doesn't that stray away from Tabs
in general? And if we don't want to actively encourage use of links, isn't this example close enough to the one for controlled mode?
As per this conversation, I'm closing this PR. I will open a new PR that focus on adding documentation to the component about why we don't recommend using it with links. |
What?
Add an example to
Tabs
to show how to use the component with links and a sample routerWhy?
This is a valid usecase and consumers could benefit from finding an example
How?
Testing Instructions
Review and test the new Storybook example, make sure it works as expected