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.

Sign up for GitHub

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

Jump to bottom

feat(tabs-extended): update to v11 styles and deprecate vertical variant #11086

Merged
merged 16 commits into from
Nov 17, 2023
Merged

feat(tabs-extended): update to v11 styles and deprecate vertical variant #11086

merged 16 commits into from
Nov 17, 2023

Conversation

emyarod
Copy link
Member

@emyarod emyarod commented Nov 2, 2023

Related Ticket(s)

Closes #10756

Description

This PR updates the tabs-extended component to use v11 Carbon styles and deprecates the vertical tabs variant

@ibmdotcom-bot
Copy link
Contributor

ibmdotcom-bot commented Nov 2, 2023 •
edited
Loading

Deploy preview created for package Carbon Web Components:
https://carbon-web-components.s3.us-east.cloud-object-storage.appdomain.cloud/deploy-previews/11086/index.html

Built with commit: 933ada06f678b71cfba69d28d2b2d6e4232a18d3

@emyarod emyarod force-pushed the 10756-feat/tabs-v2 branch 7 times, most recently from 7f95816 to 7670710 Compare November 14, 2023 20:15
@ibmdotcom-bot
Copy link
Contributor

ibmdotcom-bot commented Nov 14, 2023 •
edited
Loading

Deploy preview created for package Web Components:
https://ibmdotcom-webcomponents.s3.us-east.cloud-object-storage.appdomain.cloud/deploy-previews/11086/index.html

Built with commit: 933ada06f678b71cfba69d28d2b2d6e4232a18d3

@ibmdotcom-bot
Copy link
Contributor

ibmdotcom-bot commented Nov 14, 2023 •
edited
Loading

Deploy preview created for package Web Components (React wrapper):
https://ibmdotcom-web-components-react.s3-web.us-east.cloud-object-storage.appdomain.cloud/deploy-previews/11086/index.html

Built with commit: 933ada06f678b71cfba69d28d2b2d6e4232a18d3

@emyarod emyarod marked this pull request as ready for review November 16, 2023 15:10
@emyarod emyarod requested a review from a team as a code owner November 16, 2023 15:10
IgnacioBecerra
IgnacioBecerra reviewed Nov 16, 2023
View reviewed changes
Copy link
Contributor

@IgnacioBecerra IgnacioBecerra left a comment

Choose a reason for hiding this comment

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

Everything seems to be fine code-wise, but I noticed that the tabs titles aren't doing an ellipsis if the title is long enough. You can ignore if its a new v2 thing, but this is how they differ.

Main:
Screenshot 2023-11-16 at 7 35 13 AM

PR:
Screenshot 2023-11-16 at 7 35 19 AM

packages/styles/scss/components/tabs-extended/_tabs-extended.scss Outdated Show resolved Hide resolved
packages/web-components/src/components/tabs-extended/__stories__/tabs-extended.stories.ts Outdated Show resolved Hide resolved
...web-components/src/components/tabs-extended-media/__stories__/tabs-extended-media.stories.ts Outdated Show resolved Hide resolved
...web-components/src/components/tabs-extended-media/__stories__/tabs-extended-media.stories.ts Outdated Show resolved Hide resolved
@ariellalgilmore
Copy link
Member

I noticed the same thing @IgnacioBecerra with the tab titles... seems like an issue with cds-tab. Note on tab-width.

If we remove the the max-width it should work. Guessing maybe design might want to shorten the storybook copy. But it would be nice to make this change in cwc-v2 first
Screenshot 2023-11-16 at 8 05 11 AM

@emyarod
Copy link
Member Author

emyarod commented Nov 16, 2023

I think the card-link and card-footer elements should be correct in the story now!

for the tab title ellipses, I just had to remove the display: flex rule for the ellipses to show up properly

IgnacioBecerra
IgnacioBecerra approved these changes Nov 16, 2023
View reviewed changes
Copy link
Contributor

@IgnacioBecerra IgnacioBecerra left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @emyarod!

@ariellalgilmore
Copy link
Member

ariellalgilmore commented Nov 17, 2023 •
edited
Loading

really closee!!! I think the contained tabs height is now too small (40px) instead of (48px). Looks like it's just missing these styles, which were added for cds-tabs, but need to also be added for <c4d-tabs-extended type="contained"/> https://github.com/emyarod/carbon-for-ibm-dotcom/blob/10756-feat/tabs-v2/packages/carbon-web-components/src/components/tabs/tabs.scss#L123-L138

48px height (cwc storybook)
Screenshot 2023-11-16 at 5 24 36 PM

40px height (deploy preview)
Screenshot 2023-11-16 at 5 26 50 PM

oliviaflory
oliviaflory approved these changes Nov 17, 2023
View reviewed changes
Copy link
Contributor

@oliviaflory oliviaflory left a comment

Choose a reason for hiding this comment

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

@emyarod yesss tabs looks great!

woohoo

@kennylam kennylam merged commit 6caf5e7 into carbon-design-system:feat/carbon-for-ibm-dotcom-v2 Nov 17, 2023
6 of 19 checks passed
@emyarod emyarod deleted the 10756-feat/tabs-v2 branch November 28, 2023 19:00
kennylam pushed a commit to kennylam/carbon-for-ibm-dotcom that referenced this pull request Jun 11, 2024
@emyarod
feat(tabs-extended): update to v11 styles and deprecate vertical va…
4d4d707 
…riant (carbon-design-system#11086)

* feat(tabs): use v11 styles for extended tabs component
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@oliviaflory oliviaflory oliviaflory approved these changes

@IgnacioBecerra IgnacioBecerra IgnacioBecerra approved these changes

@ariellalgilmore ariellalgilmore Awaiting requested review from ariellalgilmore ariellalgilmore was automatically assigned from carbon-design-system/carbon-for-ibm-com-members

@annawen1 annawen1 Awaiting requested review from annawen1 annawen1 was automatically assigned from carbon-design-system/carbon-for-ibm-com-members

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@emyarod @ibmdotcom-bot @ariellalgilmore @IgnacioBecerra @oliviaflory @kennylam