Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ES|QL] Inline editing in dashboard makes the flyout to dance sometimes #167063

Closed
stratoula opened this issue Sep 22, 2023 · 7 comments · Fixed by #166169
Closed

[ES|QL] Inline editing in dashboard makes the flyout to dance sometimes #167063

stratoula opened this issue Sep 22, 2023 · 7 comments · Fixed by #166169
Assignees
Labels
bug Fixes for quality problems that affect the customer experience Feature:Dashboard Dashboard related features Feature:ES|QL ES|QL related features in Kibana Feature:Lens impact:low Addressing this issue will have a low level of impact on the quality/strength of our product. Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas Team:Visualizations Visualization editors, elastic-charts and infrastructure

Comments

@stratoula
Copy link
Contributor

stratoula commented Sep 22, 2023

Kibana version:
main (not released yet)

Describe the bug:

When you open the inline editing flyout in a dashboard it opens the push flyout. When you click one dimension it opens the second flyout and makes the dashboard dance. This happens only when the dashboard height is bigger than the screen height

This doesn't happen in Lens but I am not sure if the fix is on dashboard level or lens.

dance

@stratoula stratoula added the bug Fixes for quality problems that affect the customer experience label Sep 22, 2023
@botelastic botelastic bot added the needs-team Issues missing a team label label Sep 22, 2023
@stratoula stratoula added Team:Visualizations Visualization editors, elastic-charts and infrastructure impact:low Addressing this issue will have a low level of impact on the quality/strength of our product. and removed needs-team Issues missing a team label labels Sep 22, 2023
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-visualizations (Team:Visualizations)

@stratoula stratoula changed the title [ES|QL] [ES|QL] Inline editing in dashboard makes the flyout to dance sometimes Sep 22, 2023
@stratoula stratoula added Feature:Dashboard Dashboard related features Feature:Lens Feature:ES|QL ES|QL related features in Kibana Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas labels Sep 22, 2023
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-presentation (Team:Presentation)

@stratoula
Copy link
Contributor Author

Adding the presentation team here in case they have an idea why this is happening.

@ThomThomson
Copy link
Contributor

What a funny bug! We can have some dancing when the scrollbar is added and removed, but I figure because this is a push flyout that the scroll bar is always present?

To start, is there any chance that the 2 flyouts that stack are different widths?

@stratoula
Copy link
Contributor Author

stratoula commented Sep 22, 2023

To start, is there any chance that the 2 flyouts that stack are different widths?

No same width and we don't have it in Lens. I think you are right, it must be the dashboard scroll that creates it. I think the scrollabar disappears when the second flyout is open. 🤔

@stratoula
Copy link
Contributor Author

stratoula commented Sep 22, 2023

Which happens because the second flyout is not a push one, not sure how to fix it tbh. The problem is that the dashboard scroll disappears when the second flyout opens

@stratoula
Copy link
Contributor Author

Actually I know how to fix it! Apparently we are adding the lnsBody--overflowHidden when the dimension flyout open which we should not when it is inline editing. I am going to fix it on my inline editing PR.

@stratoula stratoula self-assigned this Sep 25, 2023
delanni pushed a commit that referenced this issue Sep 27, 2023
## Summary

Closes #166833
Closes #167063

This PR allows the inline editing of Lens panels in canvas and
dashboards.


![inline_editing](https://github.com/elastic/kibana/assets/17003240/8abb636b-7b5a-4d35-9f91-a1bf2ff963f2)

### Changes

#### Discover
- The basic UI change in Discover flyout is that we add the Cancel
button in the flyout footer (resets to the state before the flyout
changes).
- It also solves (with passing the embeddableOutput observable to the
flyout) the bug that is more prominent in this PR ([first
bullet](#162389 (comment)))


#### Dashboard
- All Lens panels have an Edit visualization panel action
- The Edit Lens action is now hidden
- A flyout opens (in the header there is a link where the users can go
to the editor)
- The by reference panels change to by value as long as the users are
editing. On apply and close, we save to the SO
- All tests have changed to accomodate this navigation change. All the
existing ones test the editor (as they used to do). I have added new
tests for testing the inline experience


#### Known issues
- If a user is currently editing a by reference visualization, doesn't
click Cancel or Apply and navigate away, the panel stays as by value
panel. The users can always go back and reset the changes and we could
not find an easy fix with Devon. We consider it as a nice to have and a
non-blocking behavior.


#### Missing
These 2 features are missing to give the full set of capabilities of the
editor to the flyout. Both are missing from ESQL editing too and are
going to be added on a follow up PR
- Chart switcher
- Suggestions (as icons and not previews)

#### Flaky tests runner 

https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/3182
(100 times)

#### Panel focus
This PR will look even better when this is merged!
#165417

### Checklist

- [x] Any text added follows [EUI's writing
guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses
sentence case text and includes [i18n
support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)
- [x]
[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)
was added for features that require explanation or tutorials
- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
- [x] Any UI touched in this PR is usable by keyboard only (learn more
about [keyboard accessibility](https://webaim.org/techniques/keyboard/))
- [x] Any UI touched in this PR does not create any new axe failures
(run axe in browser:
[FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/),
[Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US))
- [x] This renders correctly on smaller devices using a responsive
layout. (You can test this [in your
browser](https://www.browserstack.com/guide/responsive-testing-on-local-server))
- [x] This was checked for [cross-browser
compatibility](https://www.elastic.co/support/matrix#matrix_browsers)

---------

Co-authored-by: Marco Liberati <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Fixes for quality problems that affect the customer experience Feature:Dashboard Dashboard related features Feature:ES|QL ES|QL related features in Kibana Feature:Lens impact:low Addressing this issue will have a low level of impact on the quality/strength of our product. Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas Team:Visualizations Visualization editors, elastic-charts and infrastructure
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants