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

[Discover] Improve flyout interaction #193452

Closed
ryankeairns opened this issue Sep 19, 2024 · 3 comments · Fixed by #193865
Closed

[Discover] Improve flyout interaction #193452

ryankeairns opened this issue Sep 19, 2024 · 3 comments · Fixed by #193865
Assignees
Labels
Feature:Discover Discover Application impact:medium Addressing this issue will have a medium level of impact on the quality/strength of our product. Team:DataDiscovery Discover, search (e.g. data plugin and KQL), data views, saved searches. For ES|QL, use Team:ES|QL. usability

Comments

@ryankeairns
Copy link
Contributor

ryankeairns commented Sep 19, 2024

In addition to the aforementioned issue with the multiple flyouts, I don't think we want to ship with this scenario either (resizing with docs flyout in between) :/

Below, you'll see how resizing the doc viewer gets pretty funky if/when the inline docs panel is sandwiched in between.

CleanShot 2024-09-18 at 15 02 35

Originally posted by @ryankeairns in #192156 (comment)

This behavior existed prior to the referenced ES|QL docs PR. See this comment below where it happens with the 'Edit viz' flyout

#192156 (comment)

meow

@davismcphee provided some additional context:

Responded to @ryankeairns directly in Slack, but adding here too. If the ES|QL editor provides an onOpenDocsFlyout callback and some handle we can use to dismiss the flyout, e.g. through a React ref, we should be able to close the doc viewer when the docs flyout is opened and vice versa. We do similar for dismissing the doc viewer elsewhere in Discover already.

@botelastic botelastic bot added the needs-team Issues missing a team label label Sep 19, 2024
@ryankeairns ryankeairns added the Team:DataDiscovery Discover, search (e.g. data plugin and KQL), data views, saved searches. For ES|QL, use Team:ES|QL. label Sep 19, 2024
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-data-discovery (Team:DataDiscovery)

@botelastic botelastic bot removed the needs-team Issues missing a team label label Sep 19, 2024
@kertal
Copy link
Member

kertal commented Sep 23, 2024

Generally we should only allow to have a single flyout to be active, right? Problem is, every plugin has to solve this problem internally, so I wonder if there could be a solution beyond just solving it in Discover

@kertal kertal added impact:medium Addressing this issue will have a medium level of impact on the quality/strength of our product. Feature:Discover Discover Application labels Sep 23, 2024
@kertal
Copy link
Member

kertal commented Sep 23, 2024

we agreed to for now solve it in Discover

jughosta added a commit that referenced this issue Sep 27, 2024
- Closes #193452

## Summary

This PR makes sure that only one flyout is open at a time and
automatically dismisses all others.


### Checklist


- [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] This was checked for [cross-browser
compatibility](https://www.elastic.co/support/matrix#matrix_browsers)
kibanamachine pushed a commit to kibanamachine/kibana that referenced this issue Sep 27, 2024
- Closes elastic#193452

## Summary

This PR makes sure that only one flyout is open at a time and
automatically dismisses all others.

### Checklist

- [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] This was checked for [cross-browser
compatibility](https://www.elastic.co/support/matrix#matrix_browsers)

(cherry picked from commit 6fc017a)
kibanamachine referenced this issue Sep 27, 2024
…194256)

# Backport

This will backport the following commits from `main` to `8.x`:
- [[Discover] Dismiss flyouts when opening another one
(#193865)](#193865)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Julia
Rechkunova","email":"julia.rechkunova@elastic.co"},"sourceCommit":{"committedDate":"2024-09-27T07:35:31Z","message":"[Discover]
Dismiss flyouts when opening another one (#193865)\n\n- Closes
https://github.com/elastic/kibana/issues/193452\r\n\r\n##
Summary\r\n\r\nThis PR makes sure that only one flyout is open at a time
and\r\nautomatically dismisses all others.\r\n\r\n\r\n###
Checklist\r\n\r\n\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common scenarios\r\n- [x] This was
checked for
[cross-browser\r\ncompatibility](https://www.elastic.co/support/matrix#matrix_browsers)","sha":"6fc017a597fc34158313b8537f6b6a2536833cba","branchLabelMapping":{"^v9.0.0$":"main","^v8.16.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","v9.0.0","Team:DataDiscovery","backport:prev-minor","Feature:ES|QL"],"title":"[Discover]
Dismiss flyouts when opening another
one","number":193865,"url":"https://github.com/elastic/kibana/pull/193865","mergeCommit":{"message":"[Discover]
Dismiss flyouts when opening another one (#193865)\n\n- Closes
https://github.com/elastic/kibana/issues/193452\r\n\r\n##
Summary\r\n\r\nThis PR makes sure that only one flyout is open at a time
and\r\nautomatically dismisses all others.\r\n\r\n\r\n###
Checklist\r\n\r\n\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common scenarios\r\n- [x] This was
checked for
[cross-browser\r\ncompatibility](https://www.elastic.co/support/matrix#matrix_browsers)","sha":"6fc017a597fc34158313b8537f6b6a2536833cba"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/193865","number":193865,"mergeCommit":{"message":"[Discover]
Dismiss flyouts when opening another one (#193865)\n\n- Closes
https://github.com/elastic/kibana/issues/193452\r\n\r\n##
Summary\r\n\r\nThis PR makes sure that only one flyout is open at a time
and\r\nautomatically dismisses all others.\r\n\r\n\r\n###
Checklist\r\n\r\n\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common scenarios\r\n- [x] This was
checked for
[cross-browser\r\ncompatibility](https://www.elastic.co/support/matrix#matrix_browsers)","sha":"6fc017a597fc34158313b8537f6b6a2536833cba"}}]}]
BACKPORT-->

Co-authored-by: Julia Rechkunova <julia.rechkunova@elastic.co>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Discover Discover Application impact:medium Addressing this issue will have a medium level of impact on the quality/strength of our product. Team:DataDiscovery Discover, search (e.g. data plugin and KQL), data views, saved searches. For ES|QL, use Team:ES|QL. usability
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants