Fix IE specific flexbox min-height issue #66555

sulemanof · 2020-05-14T12:38:56Z

Summary

Regression after #64018

Seems this is trapped into known IE bug:
flex items ignore their parent container’s height if it’s set via the min-height property

The bug is fully described here

One of the workarounds is to replace it with the height instead.
Seems to be working fine across all browsers. Didn't find any issues.
Checked against Chrome/Firefox/Edge/IE11 across apps: Visualize, Dashboard, Graph, Canvas, Home, Dev Tools, Management.

This also fixes #66586

The fix based on adding flex-basis: auto to let container inherit the height of its descendant.

This also fixes wrong height calculation of visualization on small screens in IE:
the editors overflows a visualization

Checklist

This renders correctly on smaller devices using a responsive layout. (You can test this in your browser
This was checked for cross-browser compatibility, including a check against IE11

For maintainers

This was checked for breaking API changes and was labeled appropriately

cchaos · 2020-05-14T14:09:49Z

src/core/public/rendering/_base.scss

-  min-height: calc(100vh - #{$euiHeaderHeightCompensation});
+  // IE needs this to be "height" instead of "min-height"
+  // "min-height" causes a bug described in the next link
+  // https://github.com/philipwalton/flexbugs#3-min-height-on-a-flex-container-wont-apply-to-its-flex-items
+  height: calc(100vh - #{$euiHeaderHeightCompensation});


The problem with this switch is that the scrolls now must be managed by internal views. Not a huge deal, but we really try to keep the window scroll on the window instead of an individual element. If this problem truly only affected IE, let's keep min-height by default but change to height only for IE:

min-height: calc(100vh - #{$euiHeaderHeightCompensation}); @include internetExplorerOnly { // IE needs this to be "height" instead of "min-height" // "min-height" causes a bug described in the next link // https://github.com/philipwalton/flexbugs#3-min-height-on-a-flex-container-wont-apply-to-its-flex-items height: calc(100vh - #{$euiHeaderHeightCompensation}); }

@cchaos thanks for useful suggestion!
Done.

sulemanof · 2020-05-14T15:40:28Z

This also helped to reveal another IE specific bug in visualize, a separate issue created #66586

sulemanof · 2020-05-15T07:57:34Z

@elasticmachine merge upstream

elasticmachine · 2020-05-15T09:12:03Z

Pinging @elastic/kibana-app (Team:KibanaApp)

…nto fix/ie_flexbox_height

sulemanof · 2020-05-15T11:25:19Z

src/plugins/vis_default_editor/public/_sidebar.scss

+
+  // a hack for IE, issue: https://github.com/elastic/kibana/issues/66586
+  > .visEditorSidebar__formWrapper {
+    flex-basis: auto;


I don't think this should be wrapped with internetExplorerOnly mixin.
Checked in all browsers except Safari, works fine.
Actually, this just overwrites EUI rule, which seems was created for another IE bug 😄 :

sulemanof · 2020-05-15T11:27:12Z

src/plugins/vis_default_editor/public/_default.scss

@@ -70,8 +70,7 @@

 .visEditor__visualization {
  display: flex;
-  flex-basis: 100%;
-  flex: 1;
+  flex: 1 1 auto;


This fixes another bug:
the editors overflows a visualization on small screens, the image attached in PR description.

Can you add a comment for this one as well so it doesn't get removed?

sulemanof · 2020-05-15T11:41:28Z

I tested this across Chrome/Firefox/Edge/IE11 and didn't find any other issues, all is working fine across all visualization types. Some of examples attached below:

Chrome

small screen, area vis:

wide screen:

wide screen without data:

Firefox
Vega vis, small screen

Data table vis, playing around with different screen sizes, inner scrollbars
video recorded

IE11, the party owner 🥳

playing around with Gauge vis on different screen sizes
video recorded

even Data table vis is working fine, wow 😎
video recorded

The only browser I can't check in is Safari, so I would ask reviewers to do it 🙏

flash1293

Tested in Safari and IE didn't notice any breakages

sulemanof · 2020-05-18T07:48:36Z

@elasticmachine merge upstream

cchaos

I couldn't test in IE. I kept getting redirect errors, so I'll have to trust your testing in there. I did test, however, in Safari for you. I didn't notice any regressions in the visualize editor. 👍

cchaos · 2020-05-18T13:36:23Z

src/plugins/vis_default_editor/public/_default.scss

@@ -70,8 +70,7 @@

 .visEditor__visualization {
  display: flex;
-  flex-basis: 100%;
-  flex: 1;
+  flex: 1 1 auto;


Can you add a comment for this one as well so it doesn't get removed?

kibanamachine · 2020-05-18T22:44:22Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: c2f09fb

History

💚 Build #48572 succeeded 1805183
💚 Build #48272 succeeded 912c30b
💚 Build #48242 succeeded a01b4ce
💔 Build #47990 failed ad4132b
💚 Build #47927 succeeded 501fe48

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

* Fix IE specific flexbox min-height issue * Use internetExplorerOnly mixin * Fix other issues in IE * Add a comment Co-authored-by: Elastic Machine <[email protected]>

* master: (24 commits) [APM] agent config 'profiling_inferred_spans_min_duration' default value is '0ms' but the min value in the field is '1ms' (elastic#66886) [Canvas] Fix flaky custom element functional tests (elastic#65908) Fix IE specific flexbox min-height issue (elastic#66555) [Discover] Unskip doc link functional test (elastic#66884) Index pattern management to Kibana platform (elastic#65026) Warning and link to support matrix for IE11 (elastic#66512) [Reporting] Consolidate Server Type Defs, move some out of Legacy (elastic#66144) [SIEM] [Maps] Fixes Network Map empty tooltip (elastic#66828) [Endpoint] Encode the index of the alert in the id response (elastic#66919) [services/testSubjects] reduce retry usage, add waitForEnabled (elastic#66538) [DOCS] Identifies cloud settings for APM (elastic#66935) [SIEM][CASE] Fix configuration's page user experience (elastic#66029) Resolver: Display node 75% view submenus (elastic#64121) [SIEM] Cases] Capture timeline click and open timeline in case view (elastic#66327) [APM] Lowercase agent names so icons work (elastic#66824) [dev/cli] add support for --no-cache (elastic#66837) [Ingest Manager] Better handling of package installation problems (elastic#66541) [ML] Enhances api docs for modules endpoints (elastic#66738) dont hide errors (elastic#66764) [RFC] Global search API (elastic#64284) ...

* Fix IE specific flexbox min-height issue * Use internetExplorerOnly mixin * Fix other issues in IE * Add a comment Co-authored-by: Elastic Machine <[email protected]> Co-authored-by: Elastic Machine <[email protected]>

Fix IE specific flexbox min-height issue

501fe48

sulemanof requested review from myasonik, cchaos, flash1293, timroes and DianaDerevyankina May 14, 2020 12:45

sulemanof added v7.8.0 v7.9.0 v8.0.0 release_note:skip Skip the PR/issue when compiling release notes IE11 labels May 14, 2020

cchaos suggested changes May 14, 2020

View reviewed changes

Use internetExplorerOnly mixin

ad4132b

Merge branch 'master' into fix/ie_flexbox_height

a01b4ce

sulemanof requested a review from cchaos May 15, 2020 07:58

sulemanof marked this pull request as ready for review May 15, 2020 07:58

sulemanof requested a review from a team as a code owner May 15, 2020 07:58

sulemanof added :KibanaApp/fix-it-week Team:Visualizations Visualization editors, elastic-charts and infrastructure labels May 15, 2020

Fix other issues in IE

4e91ab3

sulemanof requested a review from a team May 15, 2020 11:13

Merge branch 'fix/ie_flexbox_height' of github.com:sulemanof/kibana i…

912c30b

…nto fix/ie_flexbox_height

sulemanof commented May 15, 2020

View reviewed changes

flash1293 approved these changes May 15, 2020

View reviewed changes

Merge branch 'master' into fix/ie_flexbox_height

1805183

cchaos approved these changes May 18, 2020

View reviewed changes

Add a comment

c2f09fb

sulemanof merged commit 428fd06 into elastic:master May 19, 2020

sulemanof deleted the fix/ie_flexbox_height branch May 19, 2020 06:50

sulemanof mentioned this pull request May 19, 2020

[7.x] Fix IE specific flexbox min-height issue (#66555) #66960

Merged

sulemanof mentioned this pull request May 19, 2020

[7.8] Fix IE specific flexbox min-height issue (#66555) #66961

Merged

sulemanof added the backported label May 19, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix IE specific flexbox min-height issue #66555

Fix IE specific flexbox min-height issue #66555

sulemanof commented May 14, 2020 •

edited

Loading

cchaos May 14, 2020

sulemanof May 14, 2020

sulemanof commented May 14, 2020

sulemanof commented May 15, 2020

elasticmachine commented May 15, 2020

sulemanof May 15, 2020

sulemanof May 15, 2020

cchaos May 18, 2020

sulemanof commented May 15, 2020

flash1293 left a comment

sulemanof commented May 18, 2020

cchaos left a comment

cchaos May 18, 2020

kibanamachine commented May 18, 2020

Fix IE specific flexbox min-height issue #66555

Fix IE specific flexbox min-height issue #66555

Conversation

sulemanof commented May 14, 2020 • edited Loading

Summary

Checklist

For maintainers

cchaos May 14, 2020

Choose a reason for hiding this comment

sulemanof May 14, 2020

Choose a reason for hiding this comment

sulemanof commented May 14, 2020

sulemanof commented May 15, 2020

elasticmachine commented May 15, 2020

sulemanof May 15, 2020

Choose a reason for hiding this comment

sulemanof May 15, 2020

Choose a reason for hiding this comment

cchaos May 18, 2020

Choose a reason for hiding this comment

sulemanof commented May 15, 2020

flash1293 left a comment

Choose a reason for hiding this comment

sulemanof commented May 18, 2020

cchaos left a comment

Choose a reason for hiding this comment

cchaos May 18, 2020

Choose a reason for hiding this comment

kibanamachine commented May 18, 2020

💚 Build Succeeded

History

sulemanof commented May 14, 2020 •

edited

Loading