fix: support collapse behavior when customizing S2 tab layout #8665
Conversation
|
Build successful! 🎉 |
|
Build successful! 🎉 |
| } | ||
| return null; | ||
|
|
||
| return ( |
There was a problem hiding this comment.
at some point, i know we refactored the collapse behavior of Tabs due to an AXE accessibility error due to where we were rendering the menu but i've run AXE on this and nothing has come up
the reason as to why i moved it is because before, if a user chose to add a custom wrapper like this:
<div>
<TabList />
<div>
<Button />
</div>
</div>
what would happen is when the tabs collapsed, the wrapper would no longer wrap around both the menu + (hidden tabs) + buttons. so in order to ensure the wrapper actually wraps the expected item, i've moved the menu + hidden tabs into here
| minWidth: 'min' | ||
| }, getAllowedOverrides())(null, props.styles)}> | ||
| tablistWrapper(null, props.styles)}> | ||
| <div className={tablist({orientation, labelBehavior, density})}> |
There was a problem hiding this comment.
we moved the HiddenTabs into the InnerTabList so that the hidden tabs take up the same space as the visible tabs. previously, if we had wrapped a tablist + buttons together (like in this example), the visible tabs would only take up space up to the buttons. however, the hidden tabs would take up the space of the entire tab container (far exceeding where the visible tabs should be visible). as a result, you could continue adding tabs even though they would be displayed behind the button before it would collaspe
| ) | ||
| }; | ||
|
|
||
| function ExampleTest(props) { |
There was a problem hiding this comment.
besides the terrible story name, do we want to have this example in our storybook docs or do we want to remove it?
There was a problem hiding this comment.
is it only an issue as a user adds them? maybe we should put a story in chromatic instead? if it requires interaction with the buttons, we can do that in a play function
There was a problem hiding this comment.
yeah, afaik, the issue only applies when the user customizes the layout. i can add a chromatic story using the play functions
There was a problem hiding this comment.
sorry, what i meant was, if you have enough tabs to overflow already and you customise the layout, will it happen? or is it only after a user presses the add?
do we need the play function? it may introduce possible flakiness or at the very least, slow down the test
There was a problem hiding this comment.
also a similar example will be on the docs page so it will be documented somewhere
There was a problem hiding this comment.
yep, but it'd be good to have a test for it still so we don't accidentally break it in the future
There was a problem hiding this comment.
ahh okay sorry i see what you mean. i tried in our storybook and if you customize the layout and have an overflow of tabs already, it'll collapse automatically. it doesn't just collapse based on an action.
but agreed, would be good to have a test of that somewhere
| ) | ||
| }; | ||
|
|
||
| function ExampleTest(props) { |
There was a problem hiding this comment.
is it only an issue as a user adds them? maybe we should put a story in chromatic instead? if it requires interaction with the buttons, we can do that in a play function
|
Build successful! 🎉 |
|
Build successful! 🎉 |
4 participants
Context (also left as a comment):
we moved the HiddenTabs into the InnerTabList so that the hidden tabs take up the same space as the visible tabs. previously, if we had wrapped a tablist + buttons together (like in this example), the visible tabs would only take up space up to the buttons. however, the hidden tabs would take up the space of the entire tab container (far exceeding where the visible tabs should be visible). as a result, you could continue adding tabs even though they would be displayed behind the button before it would collapse
Something to note is the changes to Chromatic, particularly the Vertical Tabs one. I think the new changes are correct, and I've left a comment as to why but if you feel otherwise, just lemme know
✅ Pull Request Checklist:
📝 Test Instructions:
Test the
Add Remove Tabsstory in the S2 storybook. Make sure it collapses when the Tabs have no more area to render and when you remove tabs, once there's enough space to render, the tabs will show up as individual tabs and not as a menu.It will look funky if you set the orientation to vertical but it's just that the custom styles need to be adjusted a bit
🧢 Your Project: