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

[Unified Fieldlist] Research Drag'n'Drop functionality in Discover #145083

Closed
kertal opened this issue Nov 14, 2022 · 28 comments
Closed

[Unified Fieldlist] Research Drag'n'Drop functionality in Discover #145083

kertal opened this issue Nov 14, 2022 · 28 comments
Assignees
Labels
discuss Feature:Discover Discover Application Feature:UnifiedFieldList The unified field list component used by Lens & Discover impact:medium Addressing this issue will have a medium level of impact on the quality/strength of our product. loe:needs-research This issue requires some research before it can be worked on or estimated research Team:DataDiscovery Discover, search (e.g. data plugin and KQL), data views, saved searches. For ES|QL, use Team:ES|QL.

Comments

@kertal
Copy link
Member

kertal commented Nov 14, 2022

Like Lens, we want to support Drag'n'Drop functionality in Discover.

Kapture 2022-11-14 at 11 10 04

Discover has 2 potential targets where dragging fields to makes sense

  1. Add columns to the Data table
  2. Set/Breakdown field of the Unified Histogram

This is issue is to track / discuss what needs to be done to implement Drag'n'Drop in Discover. Because it's not sufficient to just enable Drag'n'Drop in the Unified Fieldlist. It could lead to a nice POC of course.

@kertal kertal added discuss Feature:Discover Discover Application research Team:DataDiscovery Discover, search (e.g. data plugin and KQL), data views, saved searches. For ES|QL, use Team:ES|QL. 8.7 candidate Feature:UnifiedFieldList The unified field list component used by Lens & Discover labels Nov 14, 2022
@elasticmachine
Copy link
Contributor

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

@ninoslavmiskovic
Copy link
Contributor

@kertal @paulewing showed me the way Security has drag and drop today. Perhaps we can learn from this, and also we can ensure that we involve them into the shaping, so that they can adopt it 👍

@davismcphee davismcphee added loe:needs-research This issue requires some research before it can be worked on or estimated impact:medium Addressing this issue will have a medium level of impact on the quality/strength of our product. labels Jan 5, 2023
@kertal
Copy link
Member Author

kertal commented Jan 23, 2023

dear @elastic/kibana-design We would be interested, when we implement DnD in Discover, how the elements to drag should look like. Discover & Lens currently have a very different style here, Discover is optimized for adding a field by a single click, Lens is optimized for DnD. So apart from the technical challenge, how should it work in Discover?

Bildschirmfoto 2023-01-23 um 09 51 13

@jughosta
Copy link
Contributor

jughosta commented Jan 23, 2023

Experimented with integrating Drag&Drop into Discover for adding grid columns. Still a very early version. The issue is that It does not always recognises drag-events since each item in the sidebar is a button and a bit slow.

https://github.com/jughosta/kibana/pull/2/files
PR: #149577

Jan-23-2023 16-30-11

@kertal
Copy link
Member Author

kertal commented Jan 24, 2023

Experimented with integrating Drag&Drop into Discover for adding grid columns. Still a very early version. The issue is that It does not always recognises drag-events since each item in the sidebar is a button and a bit slow.

Would this change if we switch to a different markup for the sidebar elements? Isn't it also an issue in Lens since they also use buttons? What's slow in the process? Thx! First gif looks great!

@jughosta
Copy link
Contributor

@kertal Lens app is not using Eui Drag&Drop which is based on react-beautiful-dnd, they have their own implementation https://github.com/elastic/kibana/blob/main/x-pack/plugins/lens/public/drag_drop/readme.md (it would require a migration to a package/plugin if we decide to adopt it).

In the current implementation there is a delay between pulling an element and before it visually appears to be dragged. And the dropping animation takes time and it seems to be redundant.

@kertal
Copy link
Member Author

kertal commented Jan 24, 2023

@kertal Lens app is not using Eui Drag&Drop which is based on react-beautiful-dnd, they have their own implementation https://github.com/elastic/kibana/blob/main/x-pack/plugins/lens/public/drag_drop/readme.md (it would require a migration to a package/plugin if we decide to adopt it).

I mean, since we are aligning Discover & Lens it might make sense to use a common package FYI: @stratoula don't know if there have been any talks about D&D in Lens recently

In the current implementation there is a delay between pulling an element and before it visually appears to be dragged. And the dropping animation takes time and it seems to be redundant.

I guess since the EUI D&D doesn't offer many configurations, it would mean allowing more configuration of the component by consumers, if the issues @jughosta noted can be solved by configuration. It is also worth to note the react-beautiful-dnd is just in maintenance mode (FYI @elastic/eui-design)

@stratoula
Copy link
Contributor

@kertal no we don't have any talks. We didnt use the EUI DnD because it didnt cover all the cases. I think it is good idea to move it into a package and reuse it.

@breehall
Copy link
Contributor

I guess since the EUI D&D doesn't offer many configurations

@kertal can I ask what configurations would be required and which cases EUI D&D wasn't able to cover?

@kertal
Copy link
Member Author

kertal commented Jan 25, 2023

I think @jughosta noted in here POC that:

  • there is a delay between pulling an element and before it visually appears to be dragged.
  • And the dropping animation takes time and it seems to be redundant.

not sure if the first issue could be configureable , but maybe to switch the animation on/off would be possible?

@MichaelMarcialis
Copy link
Contributor

We would be interested, when we implement DnD in Discover, how the elements to drag should look like. Discover & Lens currently have a very different style here

@kertal: This strikes me as another opportunity for us to provide more consistency in field list appearance and behavior between Lens and Discover. With the addition of drag-and-drop interactions in Discover, would it make sense to style the field list items there in the same manner that we do in Lens (i.e. render them in larger size and with a higher contrast background to aid in dragging interactions)? Or are there concerns that such changes would hurt the current data density of the smaller field list items that we currently show in Discover?

Discover is optimized for adding a field by a single click, Lens is optimized for DnD.

Regarding the single-click nature of Discover's current field list items, we ultimately moved that general "add" button into the field list item popover in Lens. Additionally, after the initial efforts to consolidate the field list into a unified component, I believe Discover has also inherited this placement of a general "add" button in the popover. That said, do we still need/want an "add" button adjacent to each field list item? Or do we feel the addition of drag-and-drop and the placement of an "add" button in the field list item popover is sufficient?

Depending on the answers to the questions above, we may also want to consider adding more visual affordance to indicate to users that the field list items are now draggable (perhaps via a more identifiable drag handle). I've always feared that the field list items in Lens weren't immediately identifiable as being draggable until the user either 1) hovered/focused a field item, or 2) read the messaging provided in the Lens workspace area (telling users to explicitly use drag-and-drop). If we're adding drag-and-drop behavior into Discover without that accompanying messaging to use it in the workspace, my original fears are compounded. Would it make sense to style these items to be more obviously draggable as part of this exercise (for both Discover and Lens)?

CCing @andreadelrio, as she has much more experience with Discover than I do.

@breehall
Copy link
Contributor

there is a delay between pulling an element and before it visually appears to be dragged

We could set aside some time to investigate the delay and see what's causing it

And the dropping animation takes time and it seems to be redundant // but maybe to switch the animation on/off would be possible?

I do see an option for custom animations and no animations within react-beautiful-dnd. One potential option could be adding a new prop to EUI Drag and Drop to make this configurable for you. Working together to update EUI Drag and Drop could be a great opportunity to establish consistency and focus on accessibility for the component as well.

CC: @cee-chen @1Copenut

.

@andreadelrio
Copy link
Contributor

We would be interested, when we implement DnD in Discover, how the elements to drag should look like. Discover & Lens currently have a very different style here

@kertal: This strikes me as another opportunity for us to provide more consistency in field list appearance and behavior between Lens and Discover. With the addition of drag-and-drop interactions in Discover, would it make sense to style the field list items there in the same manner that we do in Lens (i.e. render them in larger size and with a higher contrast background to aid in dragging interactions)? Or are there concerns that such changes would hurt the current data density of the smaller field list items that we currently show in Discover?

Discover is optimized for adding a field by a single click, Lens is optimized for DnD.

Regarding the single-click nature of Discover's current field list items, we ultimately moved that general "add" button into the field list item popover in Lens. Additionally, after the initial efforts to consolidate the field list into a unified component, I believe Discover has also inherited this placement of a general "add" button in the popover. That said, do we still need/want an "add" button adjacent to each field list item? Or do we feel the addition of drag-and-drop and the placement of an "add" button in the field list item popover is sufficient?

Depending on the answers to the questions above, we may also want to consider adding more visual affordance to indicate to users that the field list items are now draggable (perhaps via a more identifiable drag handle). I've always feared that the field list items in Lens weren't immediately identifiable as being draggable until the user either 1) hovered/focused a field item, or 2) read the messaging provided in the Lens workspace area (telling users to explicitly use drag-and-drop). If we're adding drag-and-drop behavior into Discover without that accompanying messaging to use it in the workspace, my original fears are compounded. Would it make sense to style these items to be more obviously draggable as part of this exercise (for both Discover and Lens)?

CCing @andreadelrio, as she has much more experience with Discover than I do.

++ on @MichaelMarcialis' point of this being a good opportunity to bring more consistency in the field list appearance. I think we should try to align the styling with Lens (white background, dashed border on hover). If being able to display more fields without scrolling is a concern for Discover, we could consider having a compressed version of the field list item in Discover. In other words, same styling as Lens for each field (white background, dashed border on hover) but in a compressed version (i.e. smaller font size, less height). See example:

image

@MichaelMarcialis maybe this "compressed" version could become the default for Lens too? that way we don't have two versions. Maybe more density would work nicely for Lens.

++1 also on Michael's concerns about list items not being immediately identifiable as draggable. In Discover, we would not have a prominent call to action indicating users to drag and drop making this a bigger concern. At this point, I believe it would be too risky for us to remove the quick "add field as column" button that we have on the Discover field list. Instead, we should consider preserving that button even if the elements are becoming draggable (we'd need to keep that spot to show the "remove field" button anyways). cc @kertal

@1Copenut
Copy link
Contributor

I'll keep my comments to the accessibility considerations; they're generally removed from implementation details and more concerned with UX patterns.

Standardization is a good thing. It lowers the cognitive overhead for users and gets them using the product(s) faster. In this case, the key items are ensuring all users understand there's a drag and drop option, and clear instructions to use it. For screen readers those instructions should explain the keyboard controls to pick up and put down items. EUI drag and drop does a good job of announcing these changes, and has strong keyboard and screen reader interop.

The other thing I'll raise is the quick add button. If this item remains in the DOM for users who prefer to click to add than drag, we now have three potential actions that all require keyboard focus and accessible calls to action:

  1. The drag and drop button
  2. The quick add button
  3. The flyout menu button

These buttons cannot be nested inside one another, they must be sibling elements to support keyboard and screen readers at a same level as mouse navigation. I'm happy to discuss this more if anyone wants to talk through the accessible UX considerations.

@kertal
Copy link
Member Author

kertal commented Jan 30, 2023

@kertal: This strikes me as another opportunity for us to provide more consistency in field list appearance and behavior between Lens and Discover. With the addition of drag-and-drop interactions in Discover, would it make sense to style the field list items there in the same manner that we do in Lens (i.e. render them in larger size and with a higher contrast background to aid in dragging interactions)? Or are there concerns that such changes would hurt the current data density of the smaller field list items that we currently show in Discover?

@MichaelMarcialis Personally I'd recommend to keep the density similar to the current way in Discover, it would be a significant change to 1:1 use the Lens style, I'm no sure users would value having less visible fields in the sidebar in the Discover case

Discover is optimized for adding a field by a single click, Lens is optimized for DnD.

Regarding the single-click nature of Discover's current field list items, we ultimately moved that general "add" button into the field list item popover in Lens. Additionally, after the initial efforts to consolidate the field list into a unified component, I believe Discover has also inherited this placement of a general "add" button in the popover. That said, do we still need/want an "add" button adjacent to each field list item? Or do we feel the addition of drag-and-drop and the placement of an "add" button in the field list item popover is sufficient?

@MichaelMarcialis Having the add button in the popover, it would add 1 click more. A single click would still be the most efficient way to add fields. Drag'n'Drop is the add-on, we should aim to keep the simplicity.

+1 for a more identifiable drag handle

@kertal
Copy link
Member Author

kertal commented Jan 30, 2023

++ on @MichaelMarcialis' point of this being a good opportunity to bring more consistency in the field list appearance. I think we should try to align the styling with Lens (white background, dashed border on hover). If being able to display more fields without scrolling is a concern for Discover, we could consider having a compressed version of the field list item in Discover. In other words, same styling as Lens for each field (white background, dashed border on hover) but in a compressed version (i.e. smaller font size, less height). See example:

image

I really like this style! One thing I would still consider to add is the "one click" -> "add field" functionality, that's more important to Discover, and less in Lens (since there are more targets to drag to, although a primary action button like this could also be helpful)

@MichaelMarcialis maybe this "compressed" version could become the default for Lens too? that way we don't have two versions. Maybe more density would work nicely for Lens.

It also could be configurable in the Unified Field List. Personally, I would prefer "compressed", since it allows me to view more fields.

++1 also on Michael's concerns about list items not being immediately identifiable as draggable. In Discover, we would not have a prominent call to action indicating users to drag and drop making this a bigger concern. At this point, I believe it would be too risky for us to remove the quick "add field as column" button that we have on the Discover field list. Instead, we should consider preserving that button even if the elements are becoming draggable (we'd need to keep that spot to show the "remove field" button anyways). cc @kertal

Ah, yes, already been writing this, yes please, let's keep the button ...

one idea beyond ... a "quick add" button ➕ , and a "drag handle" 💧 leaving in peace next to each other?

@kertal
Copy link
Member Author

kertal commented Jan 30, 2023

The other thing I'll raise is the quick add button. If this item remains in the DOM for users who prefer to click to add than drag, we now have three potential actions that all require keyboard focus and accessible calls to action:

  1. The drag and drop button
  2. The quick add button
  3. The flyout menu button

These buttons cannot be nested inside one another, they must be sibling elements to support keyboard and screen readers at a same level as mouse navigation. I'm happy to discuss this more if anyone wants to talk through the accessible UX considerations.

Yes, I do agree we should not nest buttons, so I'm curious if we just speak of HTML markup / a11y, how should it ideally look like?

@kertal
Copy link
Member Author

kertal commented Jan 31, 2023

image

dear @andreadelrio how would it look like with a button? Maybe we could apply this style before we have drag'n'drop available, because we are currently thinking about how the "default" version of the Unified Fieldlist could look like? Thx

@andreadelrio
Copy link
Contributor

andreadelrio commented Jan 31, 2023

image

dear @andreadelrio how would it look like with a button? Maybe we could apply this style before we have drag'n'drop available, because we are currently thinking about how the "default" version of the Unified Fieldlist could look like? Thx

@kertal So we have two options here:

a) To display the add/delete button inside the field button (which we do today though it's a bit hard to tell because there's no border around it - component kbnFieldButton).

Frame 5

b) To move the add/delete button outside the field button.

Frame 4

I personally like option (b) better as I think it looks cleaner. If we're planning on adding a drag handle the field button will get busier and we can alleviate that by placing the plus/cross buttons outside. Additionally, option (b) seems to work better in Lens' case where they have an information icon.

@davismcphee
Copy link
Contributor

I personally like option (b) better as I think it looks cleaner. If we're planning on adding a drag handle the field button will get busier and we can alleviate that by placing the plus/cross buttons outside. Additionally, option (b) seems to work better in Lens' case where they have an information icon.

I haven't been very involved in this discussion so far, but ++ to option (b) -- looks much cleaner IMO.

And presumably the button markup should look something like this to address the a11y concerns raised by @1Copenut?
215871668-b7d9efc1-379e-4b42-b814-47cf6dab3f2d

@1Copenut
Copy link
Contributor

I think this looks correct for the button markup. I'm seeing 4 unique actions:

  1. The drag handle
  2. The agent.keyword that opens the flyout when pressed (plz confirm this is correct)
  3. A tooltip explainer
  4. The quick add button (adds a column with default settings)

As long as these are sibling elements and have unique labels like "Add agent Dot keyword" etc. they should be accessible. Looking at this latest design prototype, some users may ask about the similar behavior when dragging vs. clicking to quick add columns, but that's outside the scope of the original accessibility question. I think this would be an excellent candidate UI for user testing.

@kertal
Copy link
Member Author

kertal commented Feb 1, 2023

+1 for Design b
About the Drag'n'Drop, our goal is to align Discover + Lens functionality, which would mean, if we go the EUI way, it would need to cover all use cases for the needs of Lens, right @stratoula . Since Lens cases appear to be special, for me it sounds a better way to extract Lens D'n'D to a package so it can be used in the Unified Fieldlist, else we would have 2 different libs in this area

@jughosta
Copy link
Contributor

jughosta commented Feb 1, 2023

@andreadelrio

Screenshot 2023-02-01 at 15 43 44

From these both options I would prefer "a". I am afraid having a floating icon button on the right might not fit layouts of pages which is outside of the field list sidebar. Also the sidebar will have a vertical scrollbar which would interfere with the button too.

Regarding the info icon, I think we can remove it - it's not really simple to trigger it right now and its a11y text we could move to the draggable handle itself and save the space for + button inside a field item container.

Having the draggable handle on the left side of a field item could become annoying for users that they would have to reach far left on the page to grab it. Is there anything else we could try to make it also comfortable to use Drag&Drop?

@MichaelMarcialis
Copy link
Contributor

@andreadelrio, @jughosta : +1 from me on preferring direction B as well. Not nesting the "add" button inside the draggable will likely be better for usability and accessibility. That said, I've put a few quick questions/comments below:

  • Do you feel that the current grab handle icon implies only vertical dragging/reordering? Should we use a more omnidirectional icon? grabHorizontal is probably not ideal either. I imagine we'd have to make a new icon or visual.

    image

  • I like the chosen middle ground of 32px height for each draggable field (shorter than Lens' current fields, but taller than Discover's). I imagine we could use this size in Lens as well for consistency. If we decide to do so, we'll likely want to also update the height of Lens layer dimension items and drop zones to match. CCing @stratoula, @ninoslavmiskovic.

    image

  • From these both options I would prefer "a". I am afraid having a floating icon button on the right might not fit layouts of pages which is outside of the field list sidebar. Also the sidebar will have a vertical scrollbar which would interfere with the button too.

    Is the concern here in regard to having a awkward whitespace in between the field and scrollbar until a hover/focus action reveals the "add" button? If so, maybe the "add" button is something that could be animated-in instead? For example, the field could occupy the full available width of the sidebar by default, but on hover/focus, the "add" button could animate-in, shrinking the field width to allow it the necessary space.

  • Regarding the info icon, I think we can remove it - it's not really simply to trigger it right now and its a11y text we could move to the draggable handle itself and save the space for + button inside a field item container.

    Agreed. With these changes providing greater affordance on how to interact with the fields, I think we can do away with the iInCircle icon altogether on the Lens version.

@ninoslavmiskovic
Copy link
Contributor

I would recommend following :

@andreadelrio (since you will helping us on Discover UI)- Could your lead a pathway to a suggestion. Of course have a chat with @MichaelMarcialis and propose a solution forward that the WG for unified field list can discuss from with pros & cons.

I see some good alignment possibilities and suggestions.

Would that be a good approach ?

Drag & Drop requires some thinking even outside of Discover and Lens. (Keep in mind solutions will adopt this and it needs to be scalable)

@stratoula
Copy link
Contributor

About the Drag'n'Drop, our goal is to align Discover + Lens functionality, which would mean, if we go the EUI way, it would need to cover all use cases for the needs of Lens, right @stratoula . Since Lens cases appear to be special, for me it sounds a better way to extract Lens D'n'D to a package so it can be used in the Unified Fieldlist, else we would have 2 different libs in this area

@kertal sorry for missing that :) Yes I propose to use Lens DnD functionality and not having 2 different ways of dragging and dropping fields.

@andreadelrio
Copy link
Contributor

I would recommend following :

@andreadelrio (since you will helping us on Discover UI)- Could your lead a pathway to a suggestion. Of course have a chat with @MichaelMarcialis and propose a solution forward that the WG for unified field list can discuss from with pros & cons.

I see some good alignment possibilities and suggestions.

Would that be a good approach ?

Drag & Drop requires some thinking even outside of Discover and Lens. (Keep in mind solutions will adopt this and it needs to be scalable)

Quick update: Michael and I will meet on Wednesday to discuss this.

@kertal
Copy link
Member Author

kertal commented Apr 12, 2023

Closing since the implementation is already in progress, no need to keep on researching

@kertal kertal closed this as completed Apr 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discuss Feature:Discover Discover Application Feature:UnifiedFieldList The unified field list component used by Lens & Discover impact:medium Addressing this issue will have a medium level of impact on the quality/strength of our product. loe:needs-research This issue requires some research before it can be worked on or estimated research Team:DataDiscovery Discover, search (e.g. data plugin and KQL), data views, saved searches. For ES|QL, use Team:ES|QL.
Projects
None yet
Development

No branches or pull requests

10 participants