chore(Tabs): added nested Tabs demo#6645
Conversation
| <Tab eventKey={1} title={<TabTitleText>Cluster 2</TabTitleText>} tabContentId={`tabContent${1}`} /> | ||
| </Tabs> | ||
| </PageSection> | ||
| <PageSection isWidthLimited variant={PageSectionVariants.light}> |
There was a problem hiding this comment.
I don't think you want a light page section variant here. If you remove it, it will make the background of the main section grey like the core demo
There was a problem hiding this comment.
Fixed, thanks!
wise-king-sullyman
left a comment
There was a problem hiding this comment.
I've commented on a few small things that I think should be addressed.
Additionally, there's a handful of small formatting things that I think should be fixed (for example the multi-line but blockless fat arrow functions). I elected not to litter this PR with comments pointing them each out individually though; as I believe they will be best addressed by just running a formatter on it.
Since the formatters don't see JS in the markdown code blocks (or at least I haven't figured out a way to make them do so) I would suggest either having a separate demo file (like I did in #6661) or just copy/pasting this to/from a temp.js file to do the formatting fixes for you.
| Label, | ||
| LabelGroup, |
There was a problem hiding this comment.
Seems like these components aren't being used, can they be removed from the imports?
| import CheckCircleIcon from '@patternfly/react-icons/dist/js/icons/check-circle-icon'; | ||
| import InfoCircleIcon from '@patternfly/react-icons/dist/js/icons/info-circle-icon'; |
There was a problem hiding this comment.
Same with these.
| import CheckCircleIcon from '@patternfly/react-icons/dist/js/icons/check-circle-icon'; | ||
| import InfoCircleIcon from '@patternfly/react-icons/dist/js/icons/info-circle-icon'; | ||
|
|
||
| TabsOpenNestedDemo = () => { |
There was a problem hiding this comment.
We're missing the declaration for this variable
Cool thanks, I also moved it to a separate file. |
mcarrano
left a comment
There was a problem hiding this comment.
This looks good to me. @mceledonia can you also take a look?
|
These look good, are we able to offer a full width bottom border for these to sit on as an option? I recall seeing that in earlier designs and I think it would be a nice option for these nested tabs, so I want to be sure we don't miss it. Looks like this is missing from the core version too, so we can also open an issue to update both of these in the future. I don't think it needs to block this. |
What: Closes #6513