Skip to content

Add flex wrap to metric vis container - #31891

Merged
markov00 merged 2 commits into
elastic:masterfrom
markov00:fix-metric-vis-wrap
Mar 15, 2019
Merged

markov00 merged 2 commits into
elastic:masterfrom
markov00:fix-metric-vis-wrap

Conversation

@markov00

Copy link
Copy Markdown
Contributor

Summary

Fix #26400
As suggested, this PR add a flex-wrap: wrap to wrap metric values on new line if they exceed the available width.
This keep the vertically centered behaviour of single metrics, it wrap multiple metrics on new rows.

However, depending on the value or on the label length, each cell has a different width and will not render as a nice grid (as shown in the second screenshot). Adding a min-width equal to the max-content could be done in a different PR if required (unfortunately this will involves computing the max content size on the first render, and adjust on the second render the size, causing a small flick. A second solution can use external rendering of text , on canvas for example, to compute the width. I'm not aware of any better solution for that case.).

screenshot 2019-02-25 at 11 14 25

screenshot 2019-02-25 at 11 14 13

Note for QA

  • it was not checked against IE11 because the current master doesn't work with IE

Checklist

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

For maintainers

@markov00 markov00 added Feature:MetricVis Metric visualization feature v7.0.0 Team:Visualizations Team label for Lens, elastic-charts, Graph, legacy editors (TSVB, Visualize, Timelion) t// v8.0.0 v6.7.0 v7.2.0 labels Feb 25, 2019
@markov00
markov00 requested a review from a team as a code owner February 25, 2019 10:40
@elasticmachine

Copy link
Copy Markdown
Contributor

Pinging @elastic/kibana-app

@elasticmachine

Copy link
Copy Markdown
Contributor

💔 Build Failed

@markov00

Copy link
Copy Markdown
Contributor Author

failure seems unrelated:

UI Functional Tests.test/functional/apps/visualize/_input_control_vis·js.visualize app input control visualization updateFiltersOnChange is false should contain dropdown with terms aggregation results as options

Jenkins, test this

@elasticmachine

Copy link
Copy Markdown
Contributor

💚 Build Succeeded

@cchaos

cchaos commented Feb 25, 2019

Copy link
Copy Markdown
Contributor

I would highly suggest making sure to test this in IE before merging since it's notoriously bad with flex.

@elasticmachine

Copy link
Copy Markdown
Contributor

💚 Build Succeeded

@lukeelmers lukeelmers left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM, (assuming IE11 works once master is fixed). Tested Chrome OSX.

@ppisljar

ppisljar commented Mar 6, 2019

Copy link
Copy Markdown
Contributor

wow nice, code LGTM

@cchaos cchaos left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Checked on Chrome and IE11 and it works great, even if it still needs to scroll. Thx

@cchaos

cchaos commented Mar 14, 2019

Copy link
Copy Markdown
Contributor

We should get this merged in soon.

@elasticmachine

Copy link
Copy Markdown
Contributor

💚 Build Succeeded

@markov00
markov00 merged commit a52407f into elastic:master Mar 15, 2019
@timroes timroes added v6.7.1 and removed v6.7.0 labels Mar 15, 2019
markov00 added a commit to markov00/kibana that referenced this pull request Mar 15, 2019
markov00 added a commit to markov00/kibana that referenced this pull request Mar 15, 2019
@markov00

markov00 commented Mar 15, 2019 •

Copy link
Copy Markdown
Contributor Author

7.x: 5de3d18
7.0: bf42d65
6.7.1: to be merged after 6.7 release

@markov00 markov00 added v6.7.2 and removed v6.7.1 labels Apr 8, 2019
markov00 added a commit to markov00/kibana that referenced this pull request Apr 8, 2019
@markov00
markov00 deleted the fix-metric-vis-wrap branch June 10, 2019 11:24
patrykkopycinski pushed a commit to patrykkopycinski/kibana that referenced this pull request May 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Feature:MetricVis Metric visualization feature release_note:fix Team:Visualizations Team label for Lens, elastic-charts, Graph, legacy editors (TSVB, Visualize, Timelion) t// v6.7.2 v7.0.0 v7.2.0 v8.0.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants