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

fix(tabs): truncate container tab header text with ellipses #5035

Merged

Conversation

emyarod
Copy link
Member

@emyarod emyarod commented Jan 14, 2020

Closes #5016

This PR adds ellipses for overflow text in container tab headers and mobile tabs. Instead of making the anchor a flex item we keep the inherited inline-block display value and set the line height based on the anchor height and vertical padding to center the text. An alternative approach would be to wrap the text in the anchor in another element

Changelog

New

  • text truncation styles for mobile tabs

Changed

  • remove flex display property to allow text truncation styles
  • compute line height for container tabs based on tab height and vertical padding

Testing / Reviewing

Ensure that the tab variants appear correct, especially with longer tab labels

@netlify
Copy link

netlify bot commented Jan 14, 2020

Deploy preview for the-carbon-components ready!

Built with commit 13f598c

https://deploy-preview-5035--the-carbon-components.netlify.com

@netlify
Copy link

netlify bot commented Jan 14, 2020

Deploy preview for carbon-elements ready!

Built with commit 13f598c

https://deploy-preview-5035--carbon-elements.netlify.com

@netlify
Copy link

netlify bot commented Jan 14, 2020

Deploy preview for carbon-components-react ready!

Built with commit 13f598c

https://deploy-preview-5035--carbon-components-react.netlify.com

Copy link
Member

@aagonzales aagonzales left a comment

Choose a reason for hiding this comment

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

🎉 perfect!

@tw15egan
Copy link
Contributor

Oooo, nice thinking! Looks great on desktop, but when I'm testing in mobile I'm seeing some minor issues with the chevron sizing

Screen Shot 2020-01-14 at 9 57 28 AM

Screen Shot 2020-01-14 at 9 57 34 AM

@emyarod
Copy link
Member Author

emyarod commented Jan 14, 2020

looks like it's caused by the positioning of the chevron inside the padding

image

I resolved it by applying the same icon positioning logic as the listbox components

image

@emyarod emyarod force-pushed the 5016-container-tabs-text-truncation branch from 645921a to f1090b0 Compare January 14, 2020 20:01
Copy link
Contributor

@tw15egan tw15egan left a comment

Choose a reason for hiding this comment

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

Looks great! 👍

Copy link
Contributor

@abbeyhrt abbeyhrt 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!

Copy link
Contributor

@asudoh asudoh left a comment

Choose a reason for hiding this comment

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

Thanks @emyarod for jumping in!

@asudoh asudoh merged commit 12be166 into carbon-design-system:master Jan 15, 2020
@emyarod emyarod deleted the 5016-container-tabs-text-truncation branch August 30, 2021 19:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Tab] Container tabs needs text truncation.
5 participants