Skip to content
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

Add nested tabs (up to one level) #167

Merged
merged 3 commits into from
Dec 13, 2021
Merged

Add nested tabs (up to one level) #167

merged 3 commits into from
Dec 13, 2021

Conversation

julieg18
Copy link
Contributor

@julieg18 julieg18 commented Dec 9, 2021

@julieg18 julieg18 changed the title Add nested tabs Add nested tabs (up to one level) Dec 9, 2021
@shcheklein shcheklein temporarily deployed to cml-dev-add-nested-tabs-z82eiy December 9, 2021 17:35 Inactive
[key: string]: {
texts: string[]
checkedInd: number
parentText: string | null
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For now tabs can only have one parent. So you can't create a nested tab set inside a nested tab.

<toggle>
<tab>

I'm the top level tab!

<toggle>
<tab>

I'm nested tab! this works fine

<toggle>
<tab>

I'm a nested tab inside a nested tab. this will not work

</tab>
</toggle>

</tab>
</toggle>

</tab>
</toggle>

If this gets merged, I'll open an issue for allowing nested tabs go past one level.

Copy link
Contributor

@casperdcl casperdcl Dec 9, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure we'll ever need more levels; so not worth bothering (unless easy to do).

@@ -75,6 +75,25 @@ few examples for some of the most frequently used providers:
- `AWS_SECRET_ACCESS_KEY`
- `AWS_SESSION_TOKEN` **(optional)**

<toggle>
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As usual, I've placed a couple nested tabs inside /doc/cml-with-dvc for testing. This pull request will be a draft until its approved and I delete this code.

@julieg18
Copy link
Contributor Author

julieg18 commented Dec 9, 2021

I have full control of the styling for nested tabs. If anyone has some suggestions on the styling for the nested tabs, I'll be able to implement them easily :)

@julieg18 julieg18 marked this pull request as draft December 9, 2021 17:45
@julieg18 julieg18 self-assigned this Dec 9, 2021
@casperdcl
Copy link
Contributor

I have full control of the styling for nested tabs. If anyone has some suggestions on the styling for the nested tabs, I'll be able to implement them easily :)

Yes, vis. iterative/gatsby-theme-iterative#138. Could be fixed in a separate PR or this one - up to you.

Copy link
Contributor

@rogermparent rogermparent left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me! I'm thinking an implementation of nesting with the React Context API in the toggle component could support arbitrary levels, but this functions perfectly fine!

@julieg18 julieg18 temporarily deployed to cml-dev-add-nested-tabs-z82eiy December 13, 2021 20:57 Inactive
@julieg18 julieg18 marked this pull request as ready for review December 13, 2021 20:57
@julieg18 julieg18 merged commit f1dad36 into master Dec 13, 2021
@julieg18 julieg18 deleted the add-nested-tabs branch December 13, 2021 21:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

tabs: nesting
4 participants