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

Tag ellipses with Tooltip #15959

Closed
wants to merge 18 commits into from
Closed

Conversation

guidari
Copy link
Contributor

@guidari guidari commented Mar 14, 2024

Closes #15893

This PR will add the ellipses to tags that have a long text. When the ellipses is active the Tooltip will added so the user can read the whole tag

Changelog

New

  • max-inline-size to tags of 208px.
  • Tooltip added when ellipses is active in a Tag.
  • Focus is active in Read-only and Dismissible tag if the ellipses is active.
  • Added prop text to Interactive tags. That way we can keep the ellipses logic in the primitive Tag only.

Removed

  • Remove children prop from interactive tags.

Testing / Reviewing

  • Go to all Tags variants to test the Ellipses and Tooltip functionality.
  • Validate if the prop text make sense

Copy link

netlify bot commented Mar 14, 2024

Deploy Preview for v11-carbon-react ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit cc5ce74
🔍 Latest deploy log https://app.netlify.com/sites/v11-carbon-react/deploys/65f32a641d9b1c00081518e6
😎 Deploy Preview https://deploy-preview-15959--v11-carbon-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 configuration.

Copy link

netlify bot commented Mar 14, 2024

Deploy Preview for v11-carbon-react ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit 67afdde
🔍 Latest deploy log https://app.netlify.com/sites/v11-carbon-react/deploys/663cf224b90b8b000870cc1d
😎 Deploy Preview https://deploy-preview-15959--v11-carbon-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 configuration.

@guidari guidari marked this pull request as ready for review April 2, 2024 13:20
@guidari guidari requested review from a team as code owners April 2, 2024 13:20
@guidari guidari requested review from alisonjoseph and tw15egan April 2, 2024 13:20
@guidari guidari requested a review from laurenmrice April 2, 2024 18:53
Copy link
Member

@laurenmrice laurenmrice left a comment

Choose a reason for hiding this comment

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

Looking great Gui! Just a couple comments:

Tag (Stable)

Playground story
Can we show one tag here with a normal tag title length instead of the ellipsis tag? I think it’s good to show the ellipsis tag in the Read Only story like you are showing, but I think we should just show a normal tag with a desired tag title length in the Playground story.

Tag (Experimental)

Dismissible story
The dismissible tag in the small size has a misaligned close icon for most of the colors.

Screenshot 2024-04-03 at 2 19 01 PM

@guidari guidari requested a review from laurenmrice April 4, 2024 17:18
@mbgower
Copy link

mbgower commented Apr 8, 2024

I have mild concerns with making a read-only tag become interactive simply to expose the tip. How is a user to understand that is the intent, especially where the tag itself is taking focus?

Here are a few other possibilities...

  1. You make the ellipsis itself clickable/hoverable.
  2. You make the text of the tag the target, not the tag itself
  3. For either of these, you use a definition tooltip (dotted underline)

Benefits:

  • you already get the interaction for both keyboard and mouse as defined by the Definition Tooltip
  • since it is the text getting the focus not the tag, the user does not confuse the intent with that of activating the tag for some purpose (i.e., a toggle)
  • this is a more scalable solution. It doesn't just have to occur where there is truncated text in a tag. It could occur in a table cell or anywhere else that text is constrained.
  • the occurrence of an ellipsis due to text constraint could itself become the trigger for the function; In other words, whatever code you have that determines the truncation (and replaces the text with an ellipsis) would be the same piece of code that makes the ellipses a definition tooltip.

Note that it might scale better to have the phrase preceding the ellipsis be part of the definition tooltip too. That would also provide more context for accessibility (i.e. three seemingly identical ellipses on one page would instead be three different definition links that might be called 'Info about a...', 'Info about b...', 'Info about c...')

@laurenmrice
Copy link
Member

Thanks Michael for these other possibilities! I am going to share these at tomorrows design crit so we can come to a common solution for the tag overflow. I have asked Gui and Taylor to join as well if they can.

@mbgower
Copy link

mbgower commented Apr 8, 2024

Note that the proposed use of a tooltip on an operable variation of tag which also happen to be truncated seems like it should work fine; it's just when the tag is not operable that I see the main issue.

@tay1orjones
Copy link
Member

Going to mark this as a draft since we're waiting on a final revisited design spec which should be complete soon

@laurenmrice
Copy link
Member

Just wanted to note here that the design spec has been updated and reflected in the issue last week. 👍

@guidari guidari mentioned this pull request May 10, 2024
@guidari
Copy link
Contributor Author

guidari commented May 13, 2024

NEW PR #16437

@guidari guidari closed this May 13, 2024
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.

[Feature Request]: change tag title overflow content to truncate and ellipsis
4 participants