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

Break word on InfoTooltip #3532

Merged
merged 2 commits into from
Sep 26, 2017
Merged

Conversation

michellethomas
Copy link
Contributor

Someone on slack mentioned that the text was getting cut off for the expression tooltip. Because it's one long string with no word breaks, the expression text gets cut off. I'm proposing adding wordWrap: 'break-word' to the InfoTooltipWithTrigger so that the text will wrap.

If you don't think this should be the default behavior for all tooltips, I can change it to pass the styles in.

It looks like there's no bootstrap class for break-word. If there's a better way to add specific styles in, let me know.

Before:
image
After:
image

@coveralls
Copy link

coveralls commented Sep 26, 2017

Coverage Status

Coverage remained the same at 69.927% when pulling 2b2ca8b on michellethomas:tooltip_wrap_word into 7d934e7 on apache:master.

@joshwalters
Copy link
Contributor

This is great, we are seeing this issue a lot.

Copy link
Member

@mistercrunch mistercrunch left a comment

Choose a reason for hiding this comment

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

I read recently that {{}} in React are typically to be avoided as it forces PureComponent to re-render every time the parent re-renders since the object is regenerated and there's a new objectid.

The solution is to simply declare the const in module scope as in:
const tooltipStyle = { wordWrap: 'break-word' }

@mistercrunch
Copy link
Member

[out of scope for this PR] I've been thinking we should grep for {{ and clean that in our whole codebase, it should lead to less rendering. Of course the vdom magic prevents the dom from being updated, but the render and dom diffing can be significant though...

@coveralls
Copy link

Coverage Status

Coverage remained the same at 69.927% when pulling 462a489c06abe4f6a69b3d6f48324b3fc1b8a313 on michellethomas:tooltip_wrap_word into 7d934e7 on apache:master.

2 similar comments
@coveralls
Copy link

Coverage Status

Coverage remained the same at 69.927% when pulling 462a489c06abe4f6a69b3d6f48324b3fc1b8a313 on michellethomas:tooltip_wrap_word into 7d934e7 on apache:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 69.927% when pulling 462a489c06abe4f6a69b3d6f48324b3fc1b8a313 on michellethomas:tooltip_wrap_word into 7d934e7 on apache:master.

@coveralls
Copy link

coveralls commented Sep 26, 2017

Coverage Status

Coverage remained the same at 69.927% when pulling bd100ff on michellethomas:tooltip_wrap_word into 7d934e7 on apache:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage remained the same at 69.927% when pulling bd100ff on michellethomas:tooltip_wrap_word into 7d934e7 on apache:master.

@michellethomas
Copy link
Contributor Author

Interesting, thanks for the information. I had heard that defining a function when passing it through as a prop could have performance implications, but didn't know it applied to objects as well.

@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 0.20.1 labels Feb 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 0.20.1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants