Use EuiResizableContainer in visualize by sulemanof · Pull Request #86934 · elastic/kibana

sulemanof · 2020-12-24T11:47:28Z

Summary

Resolves #84343

Replace shared PanelsContainer component from kibana_reactplugin with EuiResizableContainer in Visualize default editor.

Checklist

Delete any items that are not applicable to this PR.

Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
Documentation was added for features that require explanation or tutorials
Unit or functional tests were updated or added to match the most common scenarios
Any UI touched in this PR is usable by keyboard only (learn more about keyboard accessibility)
Any UI touched in this PR does not create any new axe failures (run axe in browser: FF, Chrome)
This renders correctly on smaller devices using a responsive layout. (You can test this in your browser)
This was checked for cross-browser compatibility

For maintainers

This was checked for breaking API changes and was labeled appropriately

…ntainer_editor

…ntainer_editor2

…ntainer_editor # Conflicts: # src/plugins/vis_default_editor/public/_default.scss # src/plugins/vis_default_editor/public/_sidebar.scss # src/plugins/vis_default_editor/public/default_editor.tsx # src/plugins/vis_default_editor/public/index.scss

…ntainer_editor

sulemanof · 2021-01-25T15:00:51Z

Hey @andreadelrio
Highly need your assistance in this PR 🙏

The pr is intended to use EuiResizableContainer in visualize app.

This was working fine initially, but after the changes happened here elastic/eui#3978
I faced a problem - it stops rendering a visualization in a left panel of the container.
The rendering process from kibana side is (see the src/plugins/vis_default_editor/public/default_editor.tsx):
there is a panel with a ref in react dom tree ->

and here is an effect responsible for rendering a particular visualization in a DOM node:

I did a research and found the reason: the DOM node was unmounted from the DOM during the render, and this happened due to a subscription to resize in eui src/components/resizable_container/resizable_container.tsx

The code re-initialize the container couple of times while render (we are using lazy loading and the container doesn't have its real dimensions set initially since it is hidden by display: none under the hood of implementation and the loading spinner is shown) and removes the DOM node from the DOM, which is responsible for rendering a vis in the chain of next renderings.

Disabling the highlighted lines of code in the eui resolves the problem.

I would really appreciate your help on this issue 🙏
You could text me in slack for details if needed 🙂

andreadelrio · 2021-01-25T16:08:33Z

Pinging @thompsongl

thompsongl · 2021-01-25T19:28:11Z

elastic/eui#4447 Should get you moving again. Note that it'll take some time after it merges for the changes to 1) be part of an EUI release and 2) have that release land in Kibana. If you need it sooner to hit a deadline, let me know and we can work it out.

…ntainer_editor

sulemanof · 2021-02-08T08:15:23Z

@elasticmachine merge upstream

sulemanof · 2021-02-15T13:09:51Z

Thanks @thompsongl for helping out with elastic/eui#4447 to fix initialization problem.
The replacement is ready to be reviewed now.
The only question I have left now is the problem of usage the EUI built-in Collapsible panels for the editor case.
The reason is the editor has two directions layout: both horizontal and vertical (on phone & tablet screens - xs, s, m eui dimensions):

Screen.Recording.2021-02-15.at.16.02.46.mov

So the current implementation uses EuiResizableContainer basic horizontal layout, custom collapsible button and custom style changes on small screens.
I would like the design to take a look, maybe you'll have a new layout vision or anything else.
cc @cchaos @chandlerprall @andreadelrio

…ntainer_editor

sulemanof · 2021-02-18T14:34:52Z

@cchaos
The collapsibility on small screens is presented in the editor since at least kibana 6.x
This is used to collapse the editor panel and make a visualization "full screen".
The resizability is not available on small screens.

small screen, expanded editor panel
small screen, collapsed editor panel

Do you mean we need to get rid of this feature?
Could you please check it out to see if it meets your expectations?

In this PR I fully preserved the existing editor behavior, but in the comment above just highlighted that I can't fully use EuiResizableContainer built-in features.

elasticmachine · 2021-02-18T15:15:02Z

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

cchaos · 2021-02-18T18:36:37Z

From your before/after screens, it seems that the new implementation is the same as the previous. I'd just leave it as-is for this PR then, and then circle back with @elastic/kibana-design for help with figuring out if there should be any layout changes.

Another quick fix is that collapsible option of EuiResizableContainer comes with the option to change from horizontal to vertical, also in turn changing the direction of the icon. You may want to consider changing this direction once the screen sizes down.

sulemanof · 2021-02-19T13:46:58Z

Another quick fix is that collapsible option of EuiResizableContainer comes with the option to change from horizontal to vertical, also in turn changing the direction of the icon. You may want to consider changing this direction once the screen sizes down.

Unfortunately, EuiResizableContainer does NOT support changing horizontal/vertical layout dynamically, it provides only one direction layout initially. That's why I left the custom styles for small screens and collapse button.

cchaos · 2021-02-19T15:58:56Z

Yep, you're right. I tried to dynamically change the direction based on the window size, but it errors out. I created an EUI issue: elastic/eui#4555

ryankeairns

+1 to the idea of circling back to this in the future, post-merge and after an EUI fix becomes available.

Approving the changes with that in mind.

ryankeairns · 2021-02-19T19:34:19Z

src/plugins/vis_default_editor/public/_default.scss

@@ -1,45 +1,25 @@
 .visEditor--default {
+  height: 1px;


It would be helpful to have a comment here explaining the reason as it will undoubtedly stand out to future reviewers of this file.

sulemanof · 2021-02-22T09:11:34Z

@elasticmachine merge upstream

stratoula · 2021-02-22T10:29:08Z

@elasticmachine run elasticsearch-ci/docs

stratoula

I tested it locally on Chrome, FF and Safari and it works fine. Code LGTM 🙂

sulemanof · 2021-02-22T13:56:06Z

@elasticmachine run elasticsearch-ci/docs

kibanamachine · 2021-02-22T13:58:15Z

💚 Build Succeeded

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
`visDefaultEditor`	246.2KB	241.5KB	-4.6KB

History

💚 Build #108281 succeeded bb966cc
💚 Build #107696 succeeded 937cb31
💚 Build #106578 succeeded c7ab156
💚 Build #104592 succeeded a339e216fa580de09d102b3922d4ca6233faebdc
💔 Build #100721 failed 96c9b50

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

stratoula · 2021-02-22T14:11:02Z

@elasticmachine run elasticsearch-ci/docs

* Use EuiResizableContainer * Code cleanup * Memoize the sidebar for performance boost * Prevent mouseLeave event * Downgrade eui to 30.6.0 * Revert eui downgrade * Update styles * Render embeddable after a timeout * Fix printing * Fix resizer functional test * Create an explicit layout breakpoint * Remove extra code * Update layout styles * Add a note of using height: 1px Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

* Use EuiResizableContainer * Code cleanup * Memoize the sidebar for performance boost * Prevent mouseLeave event * Downgrade eui to 30.6.0 * Revert eui downgrade * Update styles * Render embeddable after a timeout * Fix printing * Fix resizer functional test * Create an explicit layout breakpoint * Remove extra code * Update layout styles * Add a note of using height: 1px Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

sulemanof added 17 commits December 21, 2020 14:50

Use EuiResizableContainer

7a26e3f

Code cleanup

b083e4e

Merge remote-tracking branch 'upstream/master' into feat/resizable_co…

bf93694

…ntainer_editor

Memoize the sidebar for performance boost

809e251

Prevent mouseLeave event

c4b606e

Downgrade eui to 30.6.0

8ebe852

Revert eui downgrade

82d57a7

Update styles

971b2b3

Render embeddable after a timeout

88c74ad

Merge remote-tracking branch 'upstream/master' into feat/resizable_co…

60d5640

…ntainer_editor2

Fix printing

fa7d042

Fix resizer functional test

6cc8a36

Create an explicit layout breakpoint

fdbec56

Merge remote-tracking branch 'upstream/master' into feat/resizable_co…

f8f1fc0

…ntainer_editor

Merge remote-tracking branch 'upstream/master' into feat/resizable_co…

28eb3b4

…ntainer_editor

Remove extra code

96c9b50

sulemanof force-pushed the feat/resizable_container_editor branch from 5409fac to 96c9b50 Compare January 25, 2021 14:30

thompsongl mentioned this pull request Jan 25, 2021

[EuiResizableContainer] Initialize after DOM is ready elastic/eui#4447

Merged

5 tasks

Merge remote-tracking branch 'upstream/master' into feat/resizable_co…

75d2f53

…ntainer_editor

Daniil Suleiman added 2 commits February 15, 2021 14:07

Merge branch 'master' into feat/resizable_container_editor

f567034

Update layout styles

c7ab156

sulemanof force-pushed the feat/resizable_container_editor branch from a339e21 to c7ab156 Compare February 15, 2021 12:47

sulemanof requested review from cchaos and thompsongl February 15, 2021 13:09

Merge remote-tracking branch 'upstream/master' into feat/resizable_co…

937cb31

…ntainer_editor

sulemanof marked this pull request as ready for review February 18, 2021 15:13

sulemanof requested a review from a team February 18, 2021 15:13

sulemanof requested a review from a team as a code owner February 18, 2021 15:13

sulemanof added Feature:Vis Editor Visualization editor issues release_note:skip Skip the PR/issue when compiling release notes Team:Visualizations Team label for Lens, elastic-charts, Graph, legacy editors (TSVB, Visualize, Timelion) t// v7.13.0 v8.0.0 labels Feb 18, 2021

cchaos mentioned this pull request Feb 19, 2021

[EuiResizableContainer] Allow for dynamic prop changes elastic/eui#4555

Closed

ryankeairns approved these changes Feb 19, 2021

View reviewed changes

Merge branch 'master' into feat/resizable_container_editor

bb966cc

Add a note of using height: 1px

059165c

stratoula approved these changes Feb 22, 2021

View reviewed changes

sulemanof merged commit d0a7ca2 into elastic:master Feb 22, 2021

sulemanof deleted the feat/resizable_container_editor branch February 22, 2021 14:20

sulemanof mentioned this pull request Feb 22, 2021

[7.x] Use EuiResizableContainer in visualize (#86934) #92183

Merged

stratoula mentioned this pull request Feb 25, 2021

Visualize: Metric vis pushes update button below fold #92796

Closed

Comments

Conversation

sulemanof commented Dec 24, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Checklist

For maintainers

Uh oh!

sulemanof commented Jan 25, 2021

Uh oh!

andreadelrio commented Jan 25, 2021

Uh oh!

thompsongl commented Jan 25, 2021

Uh oh!

sulemanof commented Feb 8, 2021

Uh oh!

sulemanof commented Feb 15, 2021

Uh oh!

sulemanof commented Feb 18, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

elasticmachine commented Feb 18, 2021

Uh oh!

cchaos commented Feb 18, 2021

Uh oh!

sulemanof commented Feb 19, 2021

Uh oh!

cchaos commented Feb 19, 2021

Uh oh!

ryankeairns left a comment

Choose a reason for hiding this comment

Uh oh!

ryankeairns Feb 19, 2021

Choose a reason for hiding this comment

Uh oh!

sulemanof commented Feb 22, 2021

Uh oh!

stratoula commented Feb 22, 2021

Uh oh!

stratoula left a comment

Choose a reason for hiding this comment

Uh oh!

sulemanof commented Feb 22, 2021

Uh oh!

kibanamachine commented Feb 22, 2021

💚 Build Succeeded

Metrics [docs]

Async chunks

History

Uh oh!

stratoula commented Feb 22, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

sulemanof commented Dec 24, 2020 •

edited

Loading

sulemanof commented Feb 18, 2021 •

edited

Loading