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

[Maps] add drilldown support map embeddable #75598

Merged
merged 18 commits into from
Aug 28, 2020
Merged

Conversation

nreese
Copy link
Contributor

@nreese nreese commented Aug 20, 2020

Fixes #67567

This PR adds drilldown support to map embeddable allowing users to configure drilldowns for map panels.

To test

  1. Install sample web log data
  2. go to dashboard application and create new dashboard
  3. Add map panel to dashboard
  4. Open map panel context menu and click "Create drilldown". Add one or more drill downs linking to the dashboard "[Logs] Web Traffic"
  5. Save your dashboard. Drilldowns are not active while the dashboard is in edit mode.
  6. Create filters in the map panel. Select the drill down action to navigate to the dashboard when adding filters.

About the UI

Visualization panels are much simplier than map panels. When a pie slice or heatmap square are clicked, there is only a single value or pair of values. As a result, drilldowns for visualization panels provide a popup to allow the user to select the appropriate action after clicking.
Screen Shot 2020-08-20 at 1 13 40 PM

This flow does not translate to map panels. The reason is that the map contains much more information for each element then a simple x/y axis. Users are presented with a tooltip or drawing configuration that requires pre-configuration before the filter can be generated. It is a disruptive user experience to present the user with the drilldown popover after dealing with the popover required to create the filter. As a result, this PR places the drilldown action selection as part of the initial UI to create the filter.

When creating spatial filters, if drilldowns are configured, then the user can select the drilldown in the actions select. Upon completing the drawing action, the selected drilldown will be executed with no further user interactions.
Screen Shot 2020-08-20 at 1 26 44 PM

To avoid cluttering the tooltip UI, Users can click the right arrow button to view all actions.
Screen Shot 2020-08-24 at 12 20 07 PM

Screen Shot 2020-08-20 at 1 29 55 PM

@nreese nreese added release_note:enhancement [Deprecated-Use Team:Presentation]Team:Geo Former Team Label for Geo Team. Now use Team:Presentation v8.0.0 v7.10.0 labels Aug 20, 2020
@nreese nreese requested review from jsanz and thomasneirynck August 20, 2020 19:31
@nreese nreese requested review from a team as code owners August 20, 2020 19:31
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-gis (Team:Geo)

@nreese
Copy link
Contributor Author

nreese commented Aug 20, 2020

@streamich

Can you review the changes to map_embeddable to make sure I am not abusing the ui_action_service API?

@nreese nreese requested a review from streamich August 20, 2020 19:37
@nreese nreese requested a review from a team as a code owner August 21, 2020 15:35
@nreese
Copy link
Contributor Author

nreese commented Aug 21, 2020

@elasticmachine merge upstream

@kmartastic
Copy link
Contributor

Tested this out and it looks good.
Success: Create drilldown to map.
Success: Create drilldown from map.
Observation -- need to save dashboard before navigating away.

@nreese
Copy link
Contributor Author

nreese commented Aug 24, 2020

@elasticmachine merge upstream

@elizabetdev elizabetdev self-requested a review August 24, 2020 14:00
@jsanz
Copy link
Member

jsanz commented Aug 24, 2020

Tested as well and not much to add, worked smoothly, super cool.

@nreese have you tried to put the drilldown destination below the rest of the tooltip content as used in other visualizations?

image

@nreese
Copy link
Contributor Author

nreese commented Aug 24, 2020

have you tried to put the drilldown destination below the rest of the tooltip content as used in other visualizations?

That is not possible because the filter context depends on the field. In the screen shot below, notice how you can create a filter from multiple fields. I also did not like the popup after selecting the filter. It messed up the flow the request new data from the user when they have already filled out an UI to create the filter. I like the flow where the user only has a single UI to add filters and they select the action in that flow

Screen Shot 2020-08-24 at 9 35 59 AM

Copy link
Contributor

@thomasneirynck thomasneirynck left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really cool!

These are just some minor initial comments about the general flow.

  1. When going into Edit-mode, all the drilldowns disappears from the tooltips.

This seems "off".

(I guess this helps to prevent users navigating away from the dashboard during editing, but that same restriction does not apply anywhere when the user tries to navigate elsewhere in Kibana by e.g. clicking an app-icon in the side-bar)

  1. When there are multiple drilldowns, we show the first one, and a little expando-arrow to see the full list.

I'm wondering if that's what we really want, because it seems to add a hierarchy and level of importance to these actions. Filtering > first drilldown > other drilldowns. I'm not sure if that's the case. Maybe the expando could just collect all drilldowns (clicks don't matter :) )

Note that you also can't "order" drilldowns in the drilldown editor, so the user is a little stuck when creating them (unless they remove and re-create them in the correct order).

  1. Drilldowns are not available when there are no tooltips

A user can create drilldowns when a map layer does not have tooltips.

  1. Do we want all drilldowns for every layer?

Do we want every drilldowns to show up in every tooltip of every layer?


Not sure how important these are. I think in general, it will be somewhat of a difficult concept for users to just "discover" how tooltips layers, and drilldowns relate to eachother. They need to know:

  • they have to create a tooltip first and only include fields they want to be able to create drilldowns for
  • the drilldowns need to make sense in the context of the map: --> maps with drilldowns should not contain layers where the configured drilldowns do not make sense

As work-around:

  • Would it make sense to expand the Maps-app to complain some additional info in the tooltip-card, that refers to drilldowns?
  • Would it make sense to disable drilldown-creation (if possible) if the map does not have tooltips?

Copy link
Contributor

@elizabetdev elizabetdev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the idea of just having the right arrow button to view all actions. The only thing is that when we have long lists the content seems a little bit out of place.

@nreese what do you think of separating each property with a line and align the icons to the right?

Map tooltip drilldown@2x

@nreese
Copy link
Contributor Author

nreese commented Aug 25, 2020

what do you think of separating each property with a line and align the icons to the right?

Not sure if I like all of the horizontal lines. This adds a lot of noise and visual clutter to the screen.

How about moving the buttons to the left side? Then they sit next to the field name.

@nickpeihl
Copy link
Member

Map tooltip drilldown@2x

I think the lines may help align the UI with larger documents such as the agent example. But moving the icons to the left of the field might also mitigate that so the lines might not be necessary.

@elizabetdev
Copy link
Contributor

Normally the action buttons are on the right side. And because we have an arrowRight icon that opens another screen it makes sense to be on the right side.

Option 1 - We can remove the lines. I think it makes it more difficult to know in some cases to each label property the action belongs.

Option 2 - We can add the same line color of the Layers Panel which is a very light gray

Map tooltip drilldown 2@2x

@jsanz
Copy link
Member

jsanz commented Aug 25, 2020

I like the light gray lines

@nreese
Copy link
Contributor Author

nreese commented Aug 25, 2020

Option 2 - We can add the same line color of the Layers Panel which is a very light gray

I like option 2 a lot.

@nreese
Copy link
Contributor Author

nreese commented Aug 25, 2020

@elasticmachine merge upstream

@nreese nreese requested a review from elizabetdev August 25, 2020 19:48
@elizabetdev
Copy link
Contributor

Hi @nreese,

I created a PR with a few styles that make the table rows to always get the full width: nreese#36.

tooltip-action-row-styles@2x

elizabetdev and others added 2 commits August 26, 2020 07:51
* Improving tooltip actions row styles

* Removind unecessary comment
@nreese
Copy link
Contributor Author

nreese commented Aug 26, 2020

@elasticmachine merge upstream

@nreese
Copy link
Contributor Author

nreese commented Aug 27, 2020

@elasticmachine merge upstream

Copy link
Contributor

@elizabetdev elizabetdev left a 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 PR. Tested locally and it looks good! 🎉

Copy link
Contributor

@thomasneirynck thomasneirynck left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is really slick_ addition. The interactivity with the dashboards improves leaps and bounds, especially in combination with save&return.

@@ -78,6 +78,11 @@ import { UnregisterCallback } from 'history';
import { UnwrapPromiseOrReturn } from '@kbn/utility-types';
import { UserProvidedValues } from 'src/core/server/types';

// Warning: (ae-missing-release-tag) "ACTION_GLOBAL_APPLY_FILTER" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are these warnings something we should care about?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure. I do not understand the entire auto-generated API thing. Since its only exposing a constant I figured it was good enough.

x-pack/plugins/maps/public/components/action_select.tsx Outdated Show resolved Hide resolved
@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Build metrics

@kbn/optimizer bundle module count

id value diff baseline
maps 696 +1 695

async chunks size

id value diff baseline
maps 3.3MB +16.2KB 3.3MB

page load bundle size

id value diff baseline
data 1.4MB +120.0B 1.4MB
maps 297.0KB +760.0B 296.2KB
total +880.0B

distributable file count

id value diff baseline
total 53126 +2 53124

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@nreese nreese merged commit 1e20cd4 into elastic:master Aug 28, 2020
nreese added a commit to nreese/kibana that referenced this pull request Aug 28, 2020
* [Maps] add drilldown support

* filter actions view

* use i18n getter

* remove unused changed

* tslint fixes

* more tslint changes

* clean-up and API doc changes

* update snapshots

* do not show first drilldown in tooltip

* add light grey line to seperate feature property rows

* Improving tooltip row styles (#36)

* Improving tooltip actions row styles

* Removind unecessary comment

* update snapshot and add functional test

* switch UiActionsActionDefinition to Action and remove unneeded checks

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
Co-authored-by: Elizabet Oliveira <elizabet.oliveira@elastic.co>
nreese added a commit that referenced this pull request Aug 28, 2020
* [Maps] add drilldown support

* filter actions view

* use i18n getter

* remove unused changed

* tslint fixes

* more tslint changes

* clean-up and API doc changes

* update snapshots

* do not show first drilldown in tooltip

* add light grey line to seperate feature property rows

* Improving tooltip row styles (#36)

* Improving tooltip actions row styles

* Removind unecessary comment

* update snapshot and add functional test

* switch UiActionsActionDefinition to Action and remove unneeded checks

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
Co-authored-by: Elizabet Oliveira <elizabet.oliveira@elastic.co>

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
Co-authored-by: Elizabet Oliveira <elizabet.oliveira@elastic.co>
jloleysens added a commit to jloleysens/kibana that referenced this pull request Aug 31, 2020
…s-for-710

* 'master' of github.com:elastic/kibana: (43 commits)
  [APM] Chart units don't update when toggling the chart legends (elastic#74931)
  [ILM] Add support for frozen phase in UI (elastic#75968)
  Hides advanced json for count metric (elastic#74636)
  add client-side feature usage API (elastic#75486)
  [Maps] add drilldown support map embeddable (elastic#75598)
  [Enterprise Search] Request handler refactors/enhancements + update existing routes to use shared handler (elastic#76106)
  [Resolver] model `location.search` in redux (elastic#76140)
  [APM] Prevent imports of public in server code (elastic#75979)
  fix eslint issue
  skip flaky suite (elastic#76223)
  [APM] Transaction duration anomaly alerting integration (elastic#75719)
  [Transforms] Avoid using "Are you sure" (elastic#75932)
  [Security Solution][Exceptions] - Fix bug of alerts not updating after closure from exceptions modal (elastic#76145)
  [plugin-helpers] improve 3rd party KP plugin support (elastic#75019)
  [docs/getting-started] link to yarn v1 specifically (elastic#76169)
  [Security_Solution][Resolver] Resolver loading and error state (elastic#75600)
  Fixes App Search documentation links (elastic#76133)
  Fix alerts unable to create / update when the name has trailing whitepace(s) (elastic#76079)
  [Resolver] Fix useSelector usage (elastic#76129)
  [Enterprise Search] Migrate util and components from ent-search (elastic#76051)
  ...

# Conflicts:
#	x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/components/data_tier_allocation/node_allocation.tsx
#	x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/edit_policy.tsx
#	x-pack/plugins/index_lifecycle_management/public/application/services/policies/types.ts
#	x-pack/plugins/index_lifecycle_management/public/application/services/policies/warm_phase.ts
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Maps] Add drill down support to map embeddable
8 participants