Partial rewrite of the slide tabs component #49103
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This enables us to put icons there, make the layout tighter and fit to tab list contents. It also improves accessibility by adding keyboard handling and changing the structure according to ARIA Authoring Practices Guide.
bl-nero
added
the
no-changelog
gzdunek
reviewed
Nov 19, 2024
ryanclark
approved these changes
Nov 20, 2024
|
@gzdunek I applied your feedback. Also notice that I'm planning on removing the simplified string version of tab spec to keep the code simpler, but I need to remove these from the Enterprise code first. |
gzdunek
approved these changes
Nov 22, 2024
| onChange={setActiveIndex} | ||
| activeIndex={activeIndex} | ||
| /> | ||
| </Flex> | ||
| ); | ||
| }; | ||
|
|
||
| export const Medium = () => { | ||
| const [activeIndex, setActiveIndex] = useState(0); |
There was a problem hiding this comment.
Can we have a separate state for each slider? They have a different number of steps, so you can visually simulate an 'index out of range' error :)
story.mov
| | { | ||
| /** Title displayed on the tab. */ | ||
| title: string; | ||
| ariaLabel?: never; |
There was a problem hiding this comment.
I'm wondering if we should allow ariaLabel when providing the title. However, I'm not sure how practical or useful this would be in real-world scenarios (probably no one would use it anyway).
There was a problem hiding this comment.
@gzdunek IMO, ariaLabel doesn't make any sense when the title is here.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This enables us to put icons there, make the layout tighter and fit to tab list contents. It also improves accessibility by adding keyboard handling, focus state, and changing the structure according to ARIA Authoring Practices Guide.
The small size and icons will be required for implementing the new role editor.