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

EuiListGroupItem HTML title bug #2011

Closed
bevacqua opened this issue Jun 7, 2019 · 7 comments · Fixed by #2100
Closed

EuiListGroupItem HTML title bug #2011

bevacqua opened this issue Jun 7, 2019 · 7 comments · Fixed by #2100
Assignees
Labels

Comments

@bevacqua
Copy link
Contributor

bevacqua commented Jun 7, 2019

In Cloud: (https://github.com/elastic/cloud/issues/33924)

Screen Shot 2019-06-07 at 15 04 26

In EUI docs:

Screen Shot 2019-06-07 at 16 13 03

Regardless of the fact that we use a ReactNode for the EuiListGroupItem label and I'm guessing EUI uses string most of the time, we shouldn't render something as an HTML title as well as with an EUI tooltip.

@bevacqua bevacqua added the bug label Jun 7, 2019
@thompsongl
Copy link
Contributor

This one isn't as straightforward as I thought.

label correctly accepts node values, but all internal assumptions are that the value is a string. Even if we were to hide the hover title attr when EuiTooltip is in play, we'll still run into a11y problems when a long text node is used and showToolTip is false.

We either need to 1) force showToolTip to true or 2) accept another prop (title?) that can be used in place of label when nodes are used.

Also, I'm curious how you're rendering different content in the tooltip and list item. The contents of both are just the label value from what I can tell.

@bevacqua
Copy link
Contributor Author

bevacqua commented Jul 8, 2019

All I'm saying is we should likely not pass the title attribute down to the HTML node at all? Why have both? It looks strange

@cchaos
Copy link
Contributor

cchaos commented Jul 8, 2019

The title is there in case the text is truncated.

Screen Shot 2019-07-08 at 17 41 15 PM

I do realize there is a redundancy when also adding tooltips which we can fix. However, we'll still need a way to sanitizing the children in order to be able to pass a string to the title element. I'm wondering if we can check for a type of string and pass it if it is.

@thompsongl
Copy link
Contributor

We can

  1. not pass title if showToolTip is true
  2. not pass title if label is a node

both are easy.

The other case we need to figure out is a node with content that will get truncated.

@chandlerprall
Copy link
Contributor

chandlerprall commented Jul 8, 2019

There is a 3rd option... we can ask React to render the element and feed the resulting DOM's innerText back into the title. This probably isn't as bad as it sounds, as we're already rendering the label. The title prop would be empty for a frame or two until we can inspect the text value.

@thompsongl
Copy link
Contributor

I'll take a look at making all of the above happen

@thompsongl thompsongl self-assigned this Jul 8, 2019
@cchaos
Copy link
Contributor

cchaos commented Jul 8, 2019

Option 3 is interesting and any solution to this would be a helpful global service as we truncate a lot of text that may be supplied as something other than a string. Ex: EuiFilterButton

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants