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 clipping of wide vega tooltip tables #33465

Merged
merged 3 commits into from
Mar 21, 2019

Conversation

peterschretlen
Copy link
Contributor

@peterschretlen peterschretlen commented Mar 19, 2019

Summary

Fixes #33436. An EUI tooltip has a maximum width of 256px (defined in EUI).
But vega tooltip tables (used when rendering key-value pairs of an object) can be up to 400px wide (160px for the key and 240px for the value).

When table rows became wide, they appeared truncated:
image

This change makes the max width of a vega tooltip the same as the table, giving a result like this when both the key and value exceed the max width:
image

There is 12px on either side to account for padding
image

Checklist

Use strikethroughs to remove checklist items you don't feel are applicable to this PR.

For maintainers

@peterschretlen peterschretlen requested a review from a team as a code owner March 19, 2019 02:32
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@cchaos
Copy link
Contributor

cchaos commented Mar 19, 2019

vega visualizations don't seem to load at all in IE11 (due to URL length issues)

You can test IE11 by going into Management > Advanced Settings > [Search for Session] and turn on Store Session in Local Storage

@peterschretlen
Copy link
Contributor Author

Thanks @cchaos, I changed the local storage setting and was able to verify the change in IE11.

Copy link
Contributor

@cchaos cchaos left a comment

Choose a reason for hiding this comment

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

The current implementation is going to run into issues on smaller devices. I suggest the following changes.

@timroes timroes requested a review from flash1293 March 19, 2019 22:03
@timroes timroes added Feature:Vega Vega visualizations Team:Visualizations Visualization editors, elastic-charts and infrastructure v8.0.0 v7.2.0 labels Mar 19, 2019
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app

@peterschretlen
Copy link
Contributor Author

I've revised the change - left the cell widths as they were, and changed the width of the div to be max 100%. To support mobile I've added a media query so the max width is 256px.

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

Copy link
Contributor

@cchaos cchaos left a comment

Choose a reason for hiding this comment

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

Great, I think that will work better. Just a prettier nit.

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

Copy link
Contributor

@flash1293 flash1293 left a comment

Choose a reason for hiding this comment

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

Tested in Chrome and looks good to me (smaller tooltips still seem to work fine)

@peterschretlen peterschretlen merged commit 96fda61 into elastic:master Mar 21, 2019
@peterschretlen peterschretlen deleted the fix-wide-vega-tooltips branch March 21, 2019 20:01
peterschretlen added a commit to peterschretlen/kibana that referenced this pull request Mar 21, 2019
* fix wide vega table tooltips, desktop and mobile (elastic#33436)
peterschretlen added a commit that referenced this pull request Mar 22, 2019
* fix wide vega table tooltips, desktop and mobile (#33436)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Vega Vega visualizations release_note:fix Team:Visualizations Visualization editors, elastic-charts and infrastructure v7.2.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants