[Drilldowns] Trigger picker

[Dosant]

Summary

Closes: #59569
Latest designs: #59569 (comment)
Part of: #42845
Needed for URL drilldown

Feature description

Please note, until we introduce new drilldowns (URL drilldown), user won't see any of those changes.

To play with it using example drilldowns start kibana with --run-examples flag

From dev perspective

When defining a drilldown now developer also has to specify list of triggers the drilldown supports.
Drilldown's execution context should match trigger's context, otherwise typescript will fail.

Example of hello world drilldown which works only for SELECT_RANGE_TRIGGER:

export class DashboardHelloWorldOnlyRangeSelectDrilldown
  implements Drilldown<Config, typeof SELECT_RANGE_TRIGGER, RangeSelectContext>{
 
  supportedTriggers(): Array<typeof SELECT_RANGE_TRIGGER> {
    return [SELECT_RANGE_TRIGGER];
  }

  public readonly execute = async (config: Config, context: RangeSelectContext) => {
    alert(`Hello, ${config.name}, your selected range: ${JSON.stringify(context.data.range)}`);
  };
}

SELECT_RANGE_TRIGGER is required to be repeated in 2 different places:

1. In interfaces definition as a generic. This needed for proper typescript type checks.
2. As a returned value in supportedTriggers. This will actually be used in runtime.

From user perspective

Triggers listed on a drilldown and triggers supported by embeddable type now properly work with each other:

Consider following scenarios:

Registed drilldowns	Supported triggers on current embeddable	drilldowns available?	list of drilldown type to pick from	Available triggers	Is trigger picker shown?
None	VALUE_CLICK	false	-	-	-
Drilldown (A) with only VALUE_CLICK support	RANGE_SELECT	false	-	-	-
Drilldown (A) with only VALUE_CLICK support	VALUE_CLICK	true	[A]	VALUE_CLICK	false (trigger auto picked)
Drilldown (A) with only VALUE_CLICK support, Drilldown (B) with only RANGE_SELECT support, Drilldown (C) with both VALUE_CLICK and RANGE_SELECT support	VALUE_CLICK	true	[A, C]	VALUE_CLICK	false (trigger auto picked)
Drilldown (A) with only VALUE_CLICK support	VALUE_CLICK, RANGE_SELECT	true	[A]	VALUE_CLICK	false (trigger auto picked)
Drilldown (A) with both VALUE_CLICK and RANGE_SELECT support	VALUE_CLICK, RANGE_SELECT	true	[A]	VALUE_CLICK, RANGE_SELECT	true

With trigger picker:

No trigger picker (trigger auto picked because only 1 was available):

Please note: Dashboard-to-dashboard drilldown user experience didn't change

Even-though dashboard to dashboard drilldown can be trigged both by bursting over chart or clicking on a point in a chart, user doesn't have to specify what interaction wants to attach a dashboard drilldown to.
This is because dashboard drilldown uses different high-level trigger (APPLY_FILTER_TRIGGER)

TODO:

• More unit tests (I'll pick this up after first rounds of review)
• i18n everywhere
• Discuss tooltip and help link about triggers

[elasticmachine]

Pinging @elastic/kibana-app-arch (Team:AppArch)

[ppisljar]

did a quick look over the code, LGTM

[streamich]

Checked on Chrome, LGTM. I would try to use string[] type instead of SupportedTriggers to simplify types. Also see next comment about popup title.

"Apply filter" popup title now appears (popup should be without title):

When creating a "Say hello only for range select" drilldown for the first time saw the message below, not sure if that is expected.

Then created that type of drilldown two times and both succeeded.

Maybe in the future when a drilldown supports multiple triggers we could show the trigger picker:

[Dosant]

@streamich, regarding error probably you this wasn't a clean es run? so some older Drilldowns data was there?
I didn't bother with migrations for example drilldown types. but this shouldn't happen without --run-example

[ThomThomson]

Reviewed on chrome. Changing triggers works correctly, and actions with only one trigger don't show the trigger picker as expected.

One small thing I noticed, is some inconsistencies in the mapping between supported triggers on embeddables, and the supported triggers on actions. For instance:

• A vislib bar chart where the x-axis uses a terms aggregation. In this case, the embeddable doesn't allow brushing, but still allows the 'range select' drilldowns to be added. When you add a drilldown in this state, it is impossible to trigger it.

I see that the embeddable is configured to support the brushing action, due to the date histogram functionality. I am wondering if it's even possible to get this level of granularity, where an embeddable's triggers change based on its configuration.

Overall, this issue shouldn't get in the way of merging this PR! LGTM

[Dosant]

@elasticmachine merge upstream

[Dosant]

@ThomThomson, good catch. Now supported triggers are static depending on a vis type: https://github.com/elastic/kibana/blob/master/src/plugins/vis_type_vislib/public/histogram.ts#L55
Not sure how much effort it would be to make it dynamic depending on actual configuration. I though it worth creating an issue for: #75141

[kibanamachine]

💚 Build Succeeded

• continuous-integration/kibana-ci/pull-request
• Commit: 50e3b06

Build metrics

@kbn/optimizer bundle module count

id	value	diff	baseline
dashboardEnhanced	29	+1	28
uiActionsEnhanced	129	+67	62
total		+68

page load bundle size

id	value	diff	baseline
dashboardEnhanced	185.6KB	+2.7KB	183.0KB
uiActions	214.9KB	+1.3KB	213.6KB
uiActionsEnhanced	160.3KB	+28.7KB	131.6KB
total		+32.6KB

History

• 💚 Build #68124 succeeded 3b09b68
• 💚 Build #67943 succeeded 68332c4
• 💔 Build #67921 failed 67a042e
• 💚 Build #67745 succeeded ff6b239
• 💚 Build #67512 succeeded 5b03efb

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