Prevents lengend from expanding page by tylersmalley · Pull Request #8351 · elastic/kibana

tylersmalley · 2016-09-19T22:39:01Z

Legends will overflow along the y-axis, maintaining a 100% height of the page

tylersmalley · 2016-09-19T23:12:05Z

@cjcenizal, not sure if you knew of a more preferred way of accomplishing this with flexbox instead of using absolute/relative positioning.

spalger

LGTM

spalger · 2016-09-19T23:16:25Z

src/core_plugins/kibana/public/visualize/editor/styles/_editor.less

I'm not a fan of mixing absolute positioning and flex-box, but this does work.

cjcenizal · 2016-09-19T23:38:03Z

I noticed that this works on Chrome and Safari, but not on Firefox (OS X). Digging into it some more right now...

cjcenizal · 2016-09-20T00:15:41Z

Got it... in visualize.less you need to change:

visualize {
  display: flex;
  flex-direction: column;
  height: 100%;
  width: 100%;
  overflow: auto;
  position: relative;

  .k4tip {
    white-space: pre-line;
  }

  .vis-container {
    display: flex;
    flex-direction: row;

    flex: 1 0;

to:

visualize {
  display: flex;
  flex-direction: column;
  height: 100%;
  width: 100%;
  overflow: auto;
  position: relative;

  .k4tip {
    white-space: pre-line;
  }

  .vis-container {
    display: flex;
    flex-direction: row;

    flex: 1 1 0;

The last line is the only one that changed. This fixes it for me in Firefox. Still works in Chrome and Safari. Has anyone tested IE?

thomasneirynck · 2016-09-20T00:24:10Z

@cjcenizal @tylersmalley I can test this in IE (annoyign dual boot, right now I'm on linux.... :P)

thomasneirynck · 2016-09-20T00:53:22Z

this doesn't quite work in IE as expected:

left=IE11, right=Chrome53

It does work well in Edge.

thomasneirynck · 2016-09-20T00:54:48Z

src/ui/public/visualize/visualize.less

@cjcenizal IE11 behaves exactly the other way around here.

flex: 1 0; works on IE11

flex: 1 1 0; does not work on IE11

OK, so I think i have an understanding of this problem. To break it down:

It's a known bug that IE11 ignores unitless flex-basis values in flex shorthand declarations (reference on flexbugs).

The recommended solution to this bug is to use 0%, e.g. flex: 1 1 0%.

However, Firefox has a bug where it doesn't respect 0% as it's supposed to (reference on flexbugs).

So, I think the first thing we should do is double-check the latest version of Firefox and make sure that the bug in point 3 still exists. It's possible I reproduced this bug using an older version of Firefox.

If the bug has been fixed, then we're fine using the 0% solution.

If not, then let's explore using an IE11 CSS Hack to introduce the 0% property after the regular property.

This was still an issue in FF 49. The resolution ended up being flex: 1 1 0 for all browsers except IE 11, which I used flex: 1 0

thomasneirynck · 2016-09-20T22:05:33Z

src/ui/public/visualize/visualize.less

(3) continues to be an issue on FF48

tylersmalley · 2016-09-21T16:03:44Z

**Chrome Version 53.0.2785.116 (64-bit) on OSX:**

Safari Version 9.1.1 (11601.6.17) on OSX:

Firefox Version 49.0 on OSX:

IE Version 11.0.9600.17416 on Windows 8.1:

Microsoft Edge 38.14393.0.0:

cjcenizal · 2016-09-21T16:12:45Z

@tylersmalley FYI we're not supporting IE10 afaik

cjcenizal · 2016-09-21T16:15:22Z

src/ui/public/visualize/visualize.less

I know this is kind of stupidly over-the-top, but because this hack is needed to fix a Firefox bug, I think we should elaborate, e.g.:

// Normally we would just set flex: 1 1 0%, which works as expected in IE11. // Unfortunately, a recent bug in Firefox causes this rule to be ignored, so we // have to use an IE-specific hack instead.

* Legends will overflow along the y-axis, maintaining a 100% height of the page * With these changes, space-between would at times place the text out of view Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>

spalger · 2016-09-21T16:54:23Z

LGTM

cjcenizal · 2016-09-21T16:58:43Z

LGTM2

--------- **Commit 1:** Prevents lengend from expanding page * Legends will overflow along the y-axis, maintaining a 100% height of the page * With these changes, space-between would at times place the text out of view Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co> * Original sha: bb6632f * Authored by Tyler Smalley <tyler.smalley@elastic.co> on 2016-09-19T21:29:06Z

[backport] PR #8351 to 5.0 - Prevents lengend from expanding page

[backport] PR #8351 to 5.x - Prevents lengend from expanding page

--------- **Commit 1:** Prevents lengend from expanding page * Legends will overflow along the y-axis, maintaining a 100% height of the page * With these changes, space-between would at times place the text out of view Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co> * Original sha: 943381bf8e3a523844d27c88f8d6f43b19e5cd52 [formerly bb6632f] * Authored by Tyler Smalley <tyler.smalley@elastic.co> on 2016-09-19T21:29:06Z Former-commit-id: fbf3048

[backport] PR elastic#8351 to 5.x - Prevents lengend from expanding page Former-commit-id: 9876c88

tylersmalley added review v5.0.0 labels Sep 19, 2016

tylersmalley assigned thomasneirynck Sep 19, 2016

spalger self-assigned this Sep 19, 2016

spalger approved these changes Sep 19, 2016

View reviewed changes

tylersmalley force-pushed the issues/6771 branch from cf2c33e to 4c7eb06 Compare September 20, 2016 00:20

thomasneirynck reviewed Sep 20, 2016

View reviewed changes

src/ui/public/visualize/visualize.less Outdated

Copy link
Copy Markdown

Contributor

thomasneirynck Sep 20, 2016

Choose a reason for hiding this comment

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

(3) continues to be an issue on FF48

tylersmalley force-pushed the issues/6771 branch from 4c7eb06 to 5626b3b Compare September 21, 2016 16:02

cjcenizal reviewed Sep 21, 2016

View reviewed changes

Prevents lengend from expanding page

bb6632f

* Legends will overflow along the y-axis, maintaining a 100% height of the page * With these changes, space-between would at times place the text out of view Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>

tylersmalley force-pushed the issues/6771 branch from 5626b3b to bb6632f Compare September 21, 2016 16:18

tylersmalley merged commit 66a0ce3 into elastic:master Sep 21, 2016

tylersmalley deleted the issues/6771 branch September 21, 2016 17:06

This was referenced Sep 21, 2016

[backport] PR #8351 to 5.x - Prevents lengend from expanding page #8412

Merged

[backport] PR #8351 to 4.6 - Prevents lengend from expanding page #8413

Closed

[backport] PR #8351 to 5.0 - Prevents lengend from expanding page #8414

Merged

tylersmalley added a commit that referenced this pull request Sep 21, 2016

Merge pull request #8414 from elastic/jasper/backport/8351/5.0

48725a3

[backport] PR #8351 to 5.0 - Prevents lengend from expanding page

tylersmalley added a commit that referenced this pull request Sep 21, 2016

Merge pull request #8412 from elastic/jasper/backport/8351/5.x

9876c88

[backport] PR #8351 to 5.x - Prevents lengend from expanding page

epixa added v5.0.0-rc1 v5.0.0 and removed v5.0.0 labels Oct 7, 2016

epixa mentioned this pull request Oct 8, 2016

Fix Dashboard vis height #7154

Closed

airow pushed a commit to airow/kibana that referenced this pull request Feb 16, 2017

Merge pull request elastic#8412 from elastic/jasper/backport/8351/5.x

0faa786

[backport] PR elastic#8351 to 5.x - Prevents lengend from expanding page Former-commit-id: 9876c88

Conversation

tylersmalley commented Sep 19, 2016 • edited by epixa Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tylersmalley commented Sep 19, 2016

Uh oh!

spalger left a comment

Choose a reason for hiding this comment

Uh oh!

spalger Sep 19, 2016

Choose a reason for hiding this comment

Uh oh!

cjcenizal commented Sep 19, 2016

Uh oh!

cjcenizal commented Sep 20, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

thomasneirynck commented Sep 20, 2016

Uh oh!

thomasneirynck commented Sep 20, 2016

Uh oh!

thomasneirynck Sep 20, 2016

Choose a reason for hiding this comment

Uh oh!

cjcenizal Sep 20, 2016

Choose a reason for hiding this comment

Uh oh!

tylersmalley Sep 21, 2016

Choose a reason for hiding this comment

Uh oh!

thomasneirynck Sep 20, 2016

Choose a reason for hiding this comment

Uh oh!

tylersmalley commented Sep 21, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cjcenizal commented Sep 21, 2016

Uh oh!

cjcenizal Sep 21, 2016

Choose a reason for hiding this comment

Uh oh!

spalger commented Sep 21, 2016

Uh oh!

cjcenizal commented Sep 21, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

tylersmalley commented Sep 19, 2016 •

edited by epixa

Loading

cjcenizal commented Sep 20, 2016 •

edited

Loading

tylersmalley commented Sep 21, 2016 •

edited

Loading