-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[Maps] provide drag-n-drop support to order tooltip properties #46631
Conversation
Pinging @elastic/kibana-gis |
@miukimiu Is working on mocks for this at the moment. Please coordinate with her over the UI design. |
💚 Build Succeeded |
💚 Build Succeeded |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
x-pack/legacy/plugins/maps/public/components/add_tooltip_field_popover.js
Show resolved
Hide resolved
selectedFields={selectedFields} | ||
/> | ||
</EuiTextAlign> | ||
</div> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if we could wrap this div in a collapsible element like EuiAccordion instead? It takes up a lot of real estate when someone adds a lot of fields.
Hi @nreese, Here's the design I have in mind for the tooltips: When we reach more than 5 fields a When the users click to add a new field a As you asked me, I also thought about how to rename the fields just to have for future reference: The idea is you can always rename the field but you can open the card to see the real field name. This is just an initial idea and I'm happy to improve it. 😄 You can find the Figma Prototype Here |
@miukimiu Thanks for the designs, they look great. I really like the idea of showing type icon instead of using a header. Did you see Nick's suggestion about having a way to collapse the list? Where should the controls go for that suggestion? |
IMO we may not need to collapse the element if we implement the suggested limit of five fields from @miukimiu 's mockup. |
Another suggestion could be to have all the panels collapsible and move the tooltip settings to its own card. But I think there's a reason that @nreese mentioned that the tooltip settings can't be on a different card. |
Tooltip state is currently stored in source state. Settings panels are controlled by the layer and layer state. We would need to migrate the tooltip state from the source to the layer to move the panel to its own panel. I think we need to do this in order to allow users to control which metrics and join fields are displayed in the tooltip tip. Right now, all metrics and joins show up in the tooltip but if we go the route of the side panel for overflow tooltip space then users will need to be able to specify which metric and joins are in the tooltip and the display order. Regardless, that work will be done in another PR |
@miukimiu I added a bottom border to each tooltip field instead of a box. This is more inline with the separation between metric fields for the geo-grid source that @cchaos just added. I also do not show the grab and trash icon until the hover event. This is more inline with the icons in the map legend. Thoughts? |
@nickpeihl This is ready for another look. I have fixed the flash after dragging and dropping fields. The popover now allows for the selection of multiple fields at a time. |
💚 Build Succeeded |
💚 Build Succeeded |
💚 Build Succeeded |
💚 Build Succeeded |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm! thanks.
🎊🎊🎊 |
💔 Build Failed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for merging the style fixes! 👍
Everything looks good to me!
💔 Build Failed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Small one for you @nreese. Thanks.
x-pack/legacy/plugins/maps/public/components/_tooltip_selector.scss
Outdated
Show resolved
Hide resolved
💚 Build Succeeded |
…ic#46631) * [Maps] tooltip custom labels * add drag handlers for re-ordering tooltip property order * add trash button to remove property * add jest tests for AddTooltipFieldPopover * sort EMS file tooltip properties * update TooltipSelector jest test * clean up AddTooltipFieldPopover field sorting * remove console statements * add more styles when row is getting dragged * change reorder aria label * move css changes into seperate file * allow adding multiple fields before closing popover * clear checked state on Add * update jest snapshots * use FieldIcon to display field type as icon * add bottom border to tooltip field * avoid flash after drag and drop * Tooltip styles (#32) * update TooltipSelector snapshot * replace 24px with
… (#47771) * [Maps] tooltip custom labels * add drag handlers for re-ordering tooltip property order * add trash button to remove property * add jest tests for AddTooltipFieldPopover * sort EMS file tooltip properties * update TooltipSelector jest test * clean up AddTooltipFieldPopover field sorting * remove console statements * add more styles when row is getting dragged * change reorder aria label * move css changes into seperate file * allow adding multiple fields before closing popover * clear checked state on Add * update jest snapshots * use FieldIcon to display field type as icon * add bottom border to tooltip field * avoid flash after drag and drop * Tooltip styles (#32) * update TooltipSelector snapshot * replace 24px with
Part of #46581
This PR replaces the tooltip configuration UI with a new component that allows users to use drag-n-drop to order tooltip properties. The UI changes lay the foundation needed for custom tooltip labels and support for a side bar overflow when lots of properties are requested.