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] Expanded doc flyout doesn't close when clicking outside the flyout area #160306

Closed
teresaalvarezsoler opened this issue Jun 22, 2023 · 27 comments · Fixed by #166406
Closed
Assignees
Labels
enhancement New value added to drive a business result Feature:Discover Discover Application Feature:UnifiedDocViewer Issues relating to the unified doc viewer component impact:high Addressing this issue will have a high level of impact on the quality/strength of our product. loe:small Small Level of Effort Team:DataDiscovery Discover, search (e.g. data plugin and KQL), data views, saved searches. For ES|QL, use Team:ES|QL. usability

Comments

@teresaalvarezsoler
Copy link

Problem
User needs to click in the cross icon to close the flyout for expanded documents. This is a waste of time and it is inconsistent with the rest of flyouts behavour in Discover and other Kibana apps.
image

Solution
The flyout should be closed when the user clicks anywhere outside its area.

@teresaalvarezsoler teresaalvarezsoler added Feature:Discover Discover Application enhancement New value added to drive a business result impact:low Addressing this issue will have a low 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. labels Jun 22, 2023
@elasticmachine
Copy link
Contributor

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

@timductive
Copy link
Member

I believe the decision to leave the flyout open was intentional so that the user could click through the rows in the background and have the flyout update without it closing and reopening a bunch of times. The expected behavior is that the user will want to see the details of multiple rows and the flyout should stay open and update until it is explicitly closed. It is a bit of a weird UX but not accidental. Perhaps this should go through a Design review before we just start changing it.
image

@davismcphee
Copy link
Contributor

@timductive is right that this is intentional behaviour, but it's not the first time someone has questioned this UX, so a design review could be a good idea. Using a push flyout instead was also discussed at one point, there's an issue for it here: #148902.

@teresaalvarezsoler
Copy link
Author

thanks @davismcphee, a push flyout is a better solution if the user has to interact at the same time with the table. But if this is not a must-have, then the flyout is much better for smaller screens since the table would shrink too much and may not be usable anyway. But the current behaviour is very annoying and weird for users. If the user clicks in the icon to open another document, we can just update the flyout (without having to open and close) but if the user clicks anywhere else in the screen we should close the flyout. Pleas @andreadelrio @ryankeairns provide your input here :)

@davismcphee
Copy link
Contributor

a push flyout is a better solution if the user has to interact at the same time with the table. But if this is not a must-have, then the flyout is much better for smaller screens since the table would shrink too much and may not be usable anyway.

Agreed, and the source of the push flyout issue was that we had some user requests to allow interacting with the data grid and the expanded doc at the same time without obstructing the grid, similar to the old grid that allowed expanding docs inline. The issue proposes using a push flyout at larger screen sizes and a regular flyout at smaller screen sizes to avoid the table shrinking issue you mentioned.

But a push flyout on larger screens wouldn't address the outside click concern on smaller screens anyway, so it's definitely worth getting some design input on how we could improve the experience overall 👍

@andreadelrio
Copy link
Contributor

thanks @davismcphee, a push flyout is a better solution if the user has to interact at the same time with the table. But if this is not a must-have, then the flyout is much better for smaller screens since the table would shrink too much and may not be usable anyway.

Hi all, it's also relevant to mention here that we've received feedback that the content in the Expanded document flyout should be more compact and that there's wasted white space (see screenshot below from twitter, @kertal probably remembers this). A more compact flyout would increase the viability of having it be a push one (more space left for the Document Viewer).

image

To achieve a more compact Expanded document flyout, we could:

  • Change the font in the flyout to Inter (note: we might be suggesting this change for the Document viewer itself as well as part of the ESQL initiative)
  • Clean up the layout including moving the upper pagination widget to the same row of the search bar
  • Use the floating actions we introduced for Controls and get rid of that empty space on the left

Below you'll find a prototype I put together which shows how the changes described above could improve the flyout. The flyout that you see in this prototype is almost 200px slimmer than the one we have today (given a screen of 1440px width). Additionally, I explored another idea, what if users could choose between two different display options for the flyout? push or on top.

Screen Recording 2023-06-25 at 9 16 56 PM

@teresaalvarezsoler
Copy link
Author

Wow @andreadelrio this is perfect. I really think we should implement these improvements. This is the #1 used apps in Kibana and opening docs is one of the most frequent actions. The correct UX will save tons of time and massively decrease current users' frustration.

My 2 cents on the design: the icon to change between push and normal flyout could just be a simple "pin" icon.

@davismcphee
Copy link
Contributor

@andreadelrio That prototype looks awesome! I really like the suggestions here and made a note to discuss them with the team tomorrow in our dev sync. Thanks a lot for looking into this!

@kertal
Copy link
Member

kertal commented Jun 27, 2023

+1 Looks great, btw fun fact, seems we link the first Kibana issue on GitHub a lot @teresaalvarezsoler wanted to highlight that Discover is the Number one used app ... and this was automatically linked to the first Kibana issue:

#1

@kertal
Copy link
Member

kertal commented Jun 27, 2023

Related (which we may close, because there's much more input in the current issue):

#148902

@ryankeairns
Copy link
Contributor

+++ in favor of the pushover option.
Fun side fact: our most popular viewport size is 1920w. Given that, we may even want to default to the pushover setting.

@teresaalvarezsoler
Copy link
Author

Good data point!

@kertal
Copy link
Member

kertal commented Jun 28, 2023

2 things we mentioned in our sync we should consider

  • Moving the actions to the right, we did this a while ago, and stepped back then, because it significantly increased the way users with mouse have to travel to those actions. That's why we made this part to be responsive. Icons are expanded on the right, if there's enough screen space, else there's a popover with the actions. I think this pattern is well established
  • View Single Document, View Surrounded Documents ... we might need more of such actions in the future, so it would be great to already think of a solution that doesn't need more vertical space. Maybe similar like the row actions ... just icons when there's less screen space, icons with text when there's enough screen space, horizontal alignment

@teresaalvarezsoler
Copy link
Author

@andreadelrio will better say but regarding this

Moving the actions to the right, we did this a while ago, and stepped back then, because it significantly increased the way users with mouse have to travel to those actions. That's why we made this part to be responsive

Maybe moving the floating menu to the left will solve it. There is a lot of white unused space now.

@andreadelrio
Copy link
Contributor

Moving the actions to the right, we did this a while ago, and stepped back then, because it significantly increased the way users with mouse have to travel to those actions. That's why we made this part to be responsive. Icons are expanded on the right, if there's enough screen space, else there's a popover with the actions.

@kertal do you by chance have the original issue/feedback report? I found the related PR but I don't see the actual feedback we received then. I do remember that actions on the right were an issue for people with large screens. With the proposed solution I think users should be able to resize the width of the flyout. So for example, I'd imagine a user with a large screen would opt for the push display and would adjust to the width that works better for them. In any case, seeing the original feedback would be useful to make sure we're covering all issues with this proposal.

@ryankeairns
Copy link
Contributor

ryankeairns commented Jun 29, 2023

Oooo right... pushover flyout is one thing, resizable container is another. Perhaps the latter is what we are really after on larger viewports?

This was for a different exploration, but a layout like this for wider viewports (switch to flyout on smaller sizes):
dsc_lens_embed_collapsed (4)

@kertal
Copy link
Member

kertal commented Jun 29, 2023

@andreadelrio sure, what I found is this:
https://github.com/elastic/sdh-kibana/issues/1571
There's high likely another one ... I also vaguely remember @aarju providing feedback about it

@andreadelrio
Copy link
Contributor

Oooo right... pushover flyout is one thing, resizable container is another. Perhaps the latter is what we are really after on larger viewports?

I think those are not mutually exclusive, and we should use them in tandem. So we could support a) resizable regular flyout and b) resizable push flyout.

@andreadelrio sure, what I found is this: elastic/sdh-kibana#1571 There's high likely another one ... I also vaguely remember @aarju providing feedback about it

@kertal thanks! For what is worth, I think floating actions on the left side would work nicely. We can position them in a way that values are not covered.

image

@aarju
Copy link

aarju commented Jul 3, 2023

I really like the ability to change between a push over and regular fly out, that will be a really nice feature to have. During product meetings with the Security team we've brought up the idea of a completely detachable 'pop-out' window that stays synchronized with the selected alert in the original window when reviewing alerts. That would be really nice to have for people with multiple monitors.

@davismcphee
Copy link
Contributor

I think those are not mutually exclusive, and we should use them in tandem. So we could support a) resizable regular flyout and b) resizable push flyout.

@andreadelrio I really like the idea of making the flyout resizable, but looking into this briefly I'm not sure we can use EuiResizableContainer with EuiFlyout due to how the components work currently (the resizable container wraps resizable sections while the flyout uses portals and fixed positioning). Are you aware of anywhere else in Kibana that currently does something similar? At the moment I'm thinking we'd either need EUI changes or a custom flyout implementation.

@kertal thanks! For what is worth, I think floating actions on the left side would work nicely. We can position them in a way that values are not covered.

I agree, the floating actions on the left look good to me! I think this would address the distance issues on large screens without the need for all the additional whitespace.

@andreadelrio
Copy link
Contributor

@andreadelrio I really like the idea of making the flyout resizable, but looking into this briefly I'm not sure we can use EuiResizableContainer with EuiFlyout due to how the components work currently (the resizable container wraps resizable sections while the flyout uses portals and fixed positioning). Are you aware of anywhere else in Kibana that currently does something similar? At the moment I'm thinking we'd either need EUI changes or a custom flyout implementation.

@davismcphee We're not doing anything like this anywhere in Kibana afaik. It sounds like this (resizable flyout) would be a significant effort. As a starting point, how about we implement the other improvements suggested for the flyout then?

@davismcphee davismcphee added loe:medium Medium Level of Effort impact:medium Addressing this issue will have a medium level of impact on the quality/strength of our product. Feature:UnifiedDocViewer Issues relating to the unified doc viewer component and removed impact:low Addressing this issue will have a low level of impact on the quality/strength of our product. labels Jul 12, 2023
@davismcphee
Copy link
Contributor

@davismcphee We're not doing anything like this anywhere in Kibana afaik. It sounds like this (resizable flyout) would be a significant effort. As a starting point, how about we implement the other improvements suggested for the flyout then?

@andreadelrio That makes sense to me. The other enhancements suggested would still be a big improvement over the current flyout, and we can spin off the resizable work into a new issue to track separately.

What are your thoughts on this point from @kertal, do you think there's a way to save on some vertical space here or is the stacked links the best we could do here? The plan is for this to become the standard Kibana doc viewer eventually and the available links could vary in number and text length between usages.

View Single Document, View Surrounded Documents ... we might need more of such actions in the future, so it would be great to already think of a solution that doesn't need more vertical space. Maybe similar like the row actions ... just icons when there's less screen space, icons with text when there's enough screen space, horizontal alignment

@andreadelrio
Copy link
Contributor

What are your thoughts on this point from @kertal, do you think there's a way to save on some vertical space here or is the stacked links the best we could do here? The plan is for this to become the standard Kibana doc viewer eventually and the available links could vary in number and text length between usages.

@davismcphee I think like Matthias was saying we can do icons + text if there's enough space. If there isn't (or there are too many actions) we can show icons only instead with a tooltip showing the text on hover. Also, if there are several actions available we can pick the top "x" ones and group the rest under the "more" icon.

Icon-only actions

image

Icon + text actions

image

@ninoslavmiskovic
Copy link
Contributor

@andreadelrio :

I like the suggestions with the push fly-out 👍🏻

I do not think it is necessary to offer the switch on several modes of the fly-out, let us go with the push fly-out like we are doing with ESQL.

And great with the extra UI changes.

I will make sure this enhancement gets prioritized.

@kertal
Copy link
Member

kertal commented Aug 7, 2023

@andreadelrio :

I like the suggestions with the push fly-out 👍🏻

I do not think it is necessary to offer the switch on several modes of the fly-out, let us go with the push fly-out like we are doing with ESQL.

And great with the extra UI changes.

I will make sure this enhancement gets prioritized.

@ninoslavmiskovic I'm migrating parts of this discussion to a separate issue #163284 , so just to be sure, we should decided for the user when we use the push flyout style, depending on the screen resolution, rather than make this style configurable, right?

@ninoslavmiskovic
Copy link
Contributor

@kertal - Yes. This issue should be a small UX nit that closes the fly-out when the user clicks on areas outside of it. The re-design of the UX and UI of the fly-out should be separated.

@lukasolson lukasolson added loe:small Small Level of Effort and removed loe:medium Medium Level of Effort labels Aug 10, 2023
@kertal kertal changed the title [Discover usability] Expanded doc flyout doesn't close when clicking outside the flyout area [Discover] Expanded doc flyout doesn't close when clicking outside the flyout area Aug 16, 2023
@kertal
Copy link
Member

kertal commented Aug 21, 2023

FYI this will be resolved by when #163798 is resolved

@davismcphee davismcphee added impact:high Addressing this issue will have a high level of impact on the quality/strength of our product. and removed impact:medium Addressing this issue will have a medium level of impact on the quality/strength of our product. labels Sep 8, 2023
lukasolson added a commit that referenced this issue Apr 26, 2024
## Summary

Closes #163798. 
Closes #160306

Uses `"push"` style for the document viewer flyout in Discover.

Before:

<img width="1552" alt="image"
src="https://github.com/elastic/kibana/assets/1178348/7e3e7c35-7317-44ae-b614-55eb337f742b">

After:

<img width="1552" alt="image"
src="https://github.com/elastic/kibana/assets/1178348/542b78e6-3c82-47a5-b35f-1c8635e21580">

### Checklist

- [ ] 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)
- [ ] [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

---------

Co-authored-by: Kibana Machine <[email protected]>
Co-authored-by: Stratoula Kalafateli <[email protected]>
Co-authored-by: Davis McPhee <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New value added to drive a business result Feature:Discover Discover Application Feature:UnifiedDocViewer Issues relating to the unified doc viewer component impact:high Addressing this issue will have a high level of impact on the quality/strength of our product. loe:small Small Level of Effort 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.

10 participants