Skip to content

feat(Tabs): Contained tabs on the grid#13927

Merged
kodiakhq[bot] merged 17 commits into
carbon-design-system:mainfrom
francinelucca:13353-tabs-implement-contained-tabs-on-the-css-grid
Jun 22, 2023
Merged

feat(Tabs): Contained tabs on the grid#13927
kodiakhq[bot] merged 17 commits into
carbon-design-system:mainfrom
francinelucca:13353-tabs-implement-contained-tabs-on-the-css-grid

Conversation

@francinelucca

@francinelucca francinelucca commented Jun 5, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor

Closes #13353

Adds fullWidth prop to TabList so that Tabs (container) can be aligned to the Grid

Changelog

New

  • fullWidth prop now available for contained TabList
  • 'contained fullWidth' tab story and e2e test
  • tests for fullWidth prop and added mock for useMatchMedia in Tab testing
  • 'cds-tabs--full-width' class for fullWidth contained Tabs and accompanying style
  • Grid guidance using fullWidth prop for Tabs

Testing / Reviewing

  1. Check that stories match spec: Tabs: Aligning to the Grid design guidance #13351 (comment)
  2. Check developer guidance is true and coherent: https://deploy-preview-13927--carbon-components-react.netlify.app/?path=/docs/components-tabs--contained-full-width#tabs-and-the-grid---fullwidth-prop
  3. Should have no visual/behavioral regression to default tab styles and functionality outside of fullWidth

TODO:

  • Remove Test story

@francinelucca @guidari
feat(Tabs): allow tabs to distribute width with new normalizeWidth prop
698a6ec 
Co-authored-by: Guilherme Datilio Ribeiro <guilhermedatilio@gmail.com>
@netlify

netlify Bot commented Jun 5, 2023 •
edited
Loading

Copy link
Copy Markdown

✅ Deploy Preview for carbon-components-react ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit fec0348
🔍 Latest deploy log https://app.netlify.com/sites/carbon-components-react/deploys/6494a4c4dfce8d000811c719
😎 Deploy Preview https://deploy-preview-13927--carbon-components-react.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@netlify

netlify Bot commented Jun 5, 2023 •
edited
Loading

Copy link
Copy Markdown

✅ Deploy Preview for carbon-elements ready!

Name Link
🔨 Latest commit fec0348
🔍 Latest deploy log https://app.netlify.com/sites/carbon-elements/deploys/6494a4c4e901250008b2f45c
😎 Deploy Preview https://deploy-preview-13927--carbon-elements.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

francinelucca and others added 2 commits June 7, 2023 13:03
@francinelucca
Merge branch 'main' of github.com:carbon-design-system/carbon into 13…
06eba0e 
…353-tabs-implement-contained-tabs-on-the-css-grid
@francinelucca @guidari
feat(Tabs): add ellipsis overflow to fullWidth tabs and add test stories
054642d 
Co-authored-by: Guilherme Datilio Ribeiro <guilhermedatilio@gmail.com>
@francinelucca

francinelucca commented Jun 7, 2023

Copy link
Copy Markdown
Contributor Author

We're still ironing some dev things out but this should be good for design review @kingtraceyj . You can check it out at this url: https://deploy-preview-13927--carbon-components-react.netlify.app/iframe.html?args=&id=components-tabs--contained-grid-test&viewMode=story

@francinelucca francinelucca requested a review from kingtraceyj June 7, 2023 20:44
kingtraceyj
kingtraceyj approved these changes Jun 8, 2023
View reviewed changes

@kingtraceyj kingtraceyj left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

this is working as intended! 💥

francinelucca and others added 2 commits June 12, 2023 12:47
@francinelucca
Merge branch 'main' of github.com:carbon-design-system/carbon into 13…
65b6472 
…353-tabs-implement-contained-tabs-on-the-css-grid
@francinelucca @guidari
fix(Tabs): mock useMatchMedia for tests and add fullWidth tests
9a5e81e 
Co-authored-by: Guilherme Datilio Ribeiro <guilhermedatilio@gmail.com>
@francinelucca francinelucca changed the title WIP: feat(Tabs): Contained tabs on the grid feat(Tabs): Contained tabs on the grid Jun 12, 2023
@francinelucca francinelucca marked this pull request as ready for review June 12, 2023 17:37
@francinelucca francinelucca requested a review from a team as a code owner June 12, 2023 17:37
@francinelucca francinelucca requested review from andreancardona, guidari and tw15egan and removed request for guidari June 12, 2023 17:37
@guidari guidari self-assigned this Jun 12, 2023
@francinelucca francinelucca requested a review from a team as a code owner June 12, 2023 18:35
@alisonjoseph

alisonjoseph commented Jun 15, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor

Question: are we not aligning individual tabs on the grid anymore? That was the original intent of the design spec, but maybe it changed?

Screenshot 2023-06-15 at 10 26 54 AM

This is a screen shot from the POC that we did as a comparison.
Screenshot 2023-06-15 at 10 32 37 AM

Edited to add it seems that this is intended 😅 Added some suggestions below on how we can possibly clarify the docs so they can get tabs aligned on the grid, and not just full width.

alisonjoseph
alisonjoseph suggested changes Jun 16, 2023
View reviewed changes

@alisonjoseph alisonjoseph left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is looking great! I think we can make the docs a little clearer by adding / showing an example on how to actually get the tabs on the grid by matching the column numbers. We might want to show that in the code sample on the docs also.

Comment thread packages/react/src/components/Tabs/Tabs.mdx
guidari and others added 2 commits June 16, 2023 15:13
@guidari
Merge branch 'main' into 13353-tabs-implement-contained-tabs-on-the-c…
f45617d 
…ss-grid
@francinelucca @guidari
fix: add more docs around tabs in the grid
d6dce49 
Co-authored-by: Guilherme Datilio Ribeiro <guilhermedatilio@gmail.com>
@francinelucca

francinelucca commented Jun 16, 2023

Copy link
Copy Markdown
Contributor Author

@alisonjoseph yeah we confirmed with Tracey & Lauren only the Container needs to be aligned to the Grid and if the number of columns coincide with the number of Tabs then the individual tab items will also be aligned to the Grid. We just added some extra guidance around that in the Docs, let us know what you think :)

alisonjoseph
alisonjoseph approved these changes Jun 19, 2023
View reviewed changes

@alisonjoseph alisonjoseph left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Awesome! LGTM, and a lot easier to understand now with added docs 🥳

@francinelucca
chore(Tabs): remove test story
ef33660
@francinelucca
Merge branch '13353-tabs-implement-contained-tabs-on-the-css-grid' of…
2d908a9 
… github.com:francinelucca/carbon into 13353-tabs-implement-contained-tabs-on-the-css-grid
@kodiakhq kodiakhq Bot merged commit 11c70f3 into carbon-design-system:main Jun 22, 2023
@francinelucca francinelucca deleted the 13353-tabs-implement-contained-tabs-on-the-css-grid branch June 22, 2023 20:48
@carbon-bot

carbon-bot commented Jun 23, 2023

Copy link
Copy Markdown
Contributor

Hey there! v11.32.0 was just released that references this issue/PR.

@tay1orjones tay1orjones mentioned this pull request Jun 23, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tay1orjones tay1orjones tay1orjones approved these changes

@kingtraceyj kingtraceyj kingtraceyj approved these changes

@andreancardona andreancardona Awaiting requested review from andreancardona andreancardona was automatically assigned from carbon-design-system/developers-system

@tw15egan tw15egan Awaiting requested review from tw15egan

+1 more reviewer

@alisonjoseph alisonjoseph alisonjoseph approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@guidari guidari

Labels

status: ready to merge 🎉

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Tabs: Implement contained tabs on the css grid

6 participants

@francinelucca @alisonjoseph @carbon-bot @tay1orjones @kingtraceyj @guidari