[Lens] refactor DimensionContainer and fix flyout bug #79277
Conversation
…ion Container element
mbondyra
added
Feature:Lens
release_note:skip
|
Pinging @elastic/kibana-app (Team:KibanaApp) |
mbondyra
changed the title
| .lnsDimensionContainer__footer, | ||
| .lnsDimensionContainer__header { | ||
| padding: $euiSizeS; | ||
| } | ||
|
|
||
| .lnsDimensionContainer__trigger { |
There was a problem hiding this comment.
this wasn't used anywhere
| <div | ||
| role="dialog" | ||
| aria-labelledby="lnsDimensionContainerTitle" | ||
| className={classNames('lnsDimensionContainer', { | ||
| 'lnsDimensionContainer--noAnimation': openByCreation, |
There was a problem hiding this comment.
If I understand correctly this line was responsible for directly showing the flyout without having it sliding out if it's a new dimension. Is there a special reason we don't want to do it anymore?
There was a problem hiding this comment.
@flash1293 this is a weird construct that is a legacy from popover (each trigger has its own popover/flyout).
In master, when you open a new dimension, there is an animation of opening a flyout. Then, if you configure it to the correct dimension, under the hood we close 'empty-dimension' flyout and open the 'configured dimension' flyout (it has to be different because the trigger is different). But we want the user to perceive it as seamless change, without the animation of what's really happening (close new dimension flyout, open existing dimension flyout). Adding this classname was causing the animation not to trigger in this case so it looked like it's the same flyout.
In this PR, as we have one dimensionContainer for all of the triggers, when a user switches from new dimension to configured one, it's still the same DimensionContainer so nothing has to be hacked. :)
There was a problem hiding this comment.
@mbondyra Got it, thanks for the explanation!
|
@elasticmachine merge upstream |
|
@elasticmachine merge upstream |
💚 Build SucceededMetrics [docs]async chunks size
History
To update your PR or re-run it, just comment with: |
…nes/fix-description-field * 'master' of github.com:elastic/kibana: A11y tests for user page (elastic#79199) [Ingest Pipelines] Processors editor a11y focus states (elastic#79122) [Ingest pipelines] Clean up component integration tests (elastic#78838) Drilldowns in examples (elastic#75640) Storybook and Jest cleanup (elastic#79305) adds EQL sequence rule test (elastic#79287) PR template a11y checklist item improvement (elastic#79243) [Security Solution] Adding tests for dns pipeline in the endpoint package (elastic#79177) [ML] Only adjust the bounds of SMV if annotations are visible (elastic#79210) global search to ts refs (elastic#79446) [Index management] Update TemplateDeserialized interface (elastic#78913) [Telemetry] server fetcher check all collectors ready before sending (elastic#79398) [Mappings editor] Fix app crash when selecting "other" field type (elastic#79434) [`/api/stats`] Add documentation + small improvement (elastic#79330) [Discover] "View surrounding documents" encodes spaces in filters (elastic#79283) [Lens] refactor DimensionContainer and fix flyout bug (elastic#79277) # Conflicts: # x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/components/pipeline_processors_editor_item/inline_text_input.tsx # x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/components/processors_tree/components/private_tree.tsx
Summary
Fixes #79255
Refactors DimensionContainer by separating triggers from flyout. Before each trigger has its flyout and it made its state very complicated to manage. Here I've changed it so that we have multiple triggers and only one flyout.
I know this one can be tricky but I would appreciate a quick feedback to ship it in 7.10 before feature freeze as it is also fix of a bug @flash1293 @dej611 @cchaos @wylieconlon 🙏