[Lens] Make open in discover drilldown work #131237

flash1293 · 2022-04-29T13:20:22Z

Adds a drilldown "open in discover" to Lens panels - only config option is whether or not discover is opened in a new tab.

The drilldown becomes active for the filter trigger and basically does the same thing as the panel action "Open in Discover", but it takes the context of the filter click into account as well

Tech details

As it's sharing almost all logic with the existing action, I moved these parts into open_in_discover helpers, which are called from the drilldown as well as the action definition, with the following additions:

"Open in new tab" can be configured
filters and timerange are passed in and spread into the locator call along with the filters and timerange from getViewUnderlyingData

To be able to only show the drilldown in the form flyout for Lens embeddables, I added an isConfigurable callback to both the drilldown and actionfactory interfaces and call it before rendering the keypad menu to only show it in case the current embeddable is a Lens embeddable.

Open question

Which icon to use? The screenshots are outdated, I went with discoverApp but I'm not sure - it would be nice to stay consistent with the panel action but the icon only makes sense for "open in new tab"

…illdown

elasticmachine · 2022-05-02T14:08:21Z

Pinging @elastic/kibana-vis-editors @elastic/kibana-vis-editors-external (Team:VisEditors)

stratoula · 2022-05-03T07:01:25Z

@elasticmachine merge upstream

stratoula · 2022-05-03T07:10:46Z

@flash1293 I create a drilldown with the open in new tab switch off

but when I use it , it opens Discover in a new tab. Am I missing something?

flash1293 · 2022-05-03T09:29:54Z

@stratoula Good catch, something I missed during refactoring. SHould be fixed now.

stratoula · 2022-05-03T09:50:19Z

@flash1293 it is fixed thanx! Another question. I can see the new drilldown on aggregation based visualizations.

I can configure it, but it doesn't work (which makes sense). Can we enable this only for Lens panels?

flash1293 · 2022-05-03T10:16:55Z

That's a good point @stratoula. @ghudgins what do think about this? Would it be OK if the drilldown is showing up even if the panel won't be able to handle it?

flash1293 · 2022-05-03T10:38:29Z

So, I looked into how this drilldown (which will only work in the Lens context) can be ignored otherwise and it's not that hard to add a callback for it. @vadimkibana what do you think about this extension? I'm sure it will be useful for other things as well.

drewdaemon

Nice work! Left a couple of questions. Code review only.

x-pack/plugins/lens/public/plugin.ts

x-pack/plugins/lens/public/trigger_actions/open_in_discover_action.test.ts

x-pack/plugins/lens/public/trigger_actions/open_in_discover_action.ts

x-pack/plugins/lens/public/trigger_actions/open_in_discover_helpers.ts

…illdown

stratoula

This works great Joe! I find it super useful to be able to add drilldowns specific per embeddable type so my +1 on this approach!

vadimkibana · 2022-05-05T10:27:44Z

Shall we use Discover icon?

vadimkibana · 2022-05-05T10:28:01Z

I guess it closes this issue: #60227

flash1293 · 2022-05-05T10:35:24Z

@vadimkibana I changed it to the discover icon in the code already, the screenshots are outdated.

vadimkibana · 2022-05-05T10:36:46Z

...blic/drilldowns/drilldown_manager/containers/action_factory_picker/action_factory_picker.tsx

@@ -14,14 +14,17 @@ export const ActionFactoryPicker: React.FC = ({}) => {
  const drilldowns = useDrilldownManager();
  const factory = drilldowns.useActionFactory();
  const context = React.useMemo(() => drilldowns.getActionFactoryContext(), [drilldowns]);
+  const applicableFactories = drilldowns.deps.actionFactories.filter((actionFactory) =>
+    actionFactory.isConfigurable(context)


Shouldn't this be

Suggested change

actionFactory.isConfigurable(context)

actionFactory.isCompatible(context)

and we would not need the new isConfigurable property? Or am I missing something. What is the difference between isCompatible and isConfigurable for the action factory?

@vadimkibana what do you think about this extension? I'm sure it will be useful for other things as well.

I'm trying to understand how isConfigurable is different from the existing isCompatible.

I wasn't sure about the semantics of isCompatible in this place, but it seems like it's coming from here

kibana/src/plugins/ui_actions/public/util/presentable.ts

Line 56 in 3730dd0

* Returns a promise that resolves to true if this item is compatible given

in this case it's a pretty good fit. Will adjust

…illdown

flash1293 · 2022-05-06T07:52:02Z

@vadimkibana Found a solution I'm happy with - most of the logic is in the state manager now. Could you check out the PR again?

kibana-ci · 2022-05-09T07:16:45Z

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`lens`	765	767	+2

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id	before	after	diff
`uiActionsEnhanced`	141	142	+1

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
`uiActionsEnhanced`	126.7KB	127.1KB	+472.0B

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
`lens`	30.3KB	32.5KB	+2.1KB
`uiActionsEnhanced`	22.3KB	22.3KB	+23.0B
total			+2.2KB

Unknown metric groups

API count

id	before	after	diff
`uiActionsEnhanced`	203	205	+2

ESLint disabled line counts

id	before	after	diff
`lens`	29	30	+1

References to deprecated APIs

id	before	after	diff
`lens`	14	20	+6

Total ESLint disabled count

id	before	after	diff
`lens`	31	32	+1

History

💚 Build #42830 succeeded be542fd
💚 Build #42145 succeeded 718d779
💚 Build #42022 succeeded 1854a21
💚 Build #42002 succeeded 7225c50
💛 Build #41967 was flaky e8aac1a
💚 Build #41740 succeeded c5707ee

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

* make open in discover drilldown work * cleanup and tests * fix test * fix icon * fix type * fix open in new tab * fix open in new tab * fix test * make it possible to filter out drilldowns from list based on context * review comments * remove isConfigurable from the actionfactory Co-authored-by: Kibana Machine <[email protected]> Co-authored-by: Stratoula Kalafateli <[email protected]>

make open in discover drilldown work

452d8f9

flash1293 added the skip-ci label Apr 29, 2022

flash1293 added 3 commits May 2, 2022 08:52

Merge remote-tracking branch 'upstream/main' into open-in-discover-dr…

2c0d1cd

…illdown

cleanup and tests

3174454

fix test

c426c54

flash1293 added release_note:enhancement Team:Visualizations Visualization editors, elastic-charts and infrastructure Feature:Lens backport:skip This commit does not require backporting v8.3.0 and removed skip-ci labels May 2, 2022

flash1293 added 2 commits May 2, 2022 12:29

fix icon

f6dd31d

fix type

c5707ee

flash1293 marked this pull request as ready for review May 2, 2022 14:08

flash1293 requested a review from a team as a code owner May 2, 2022 14:08

Merge branch 'main' into open-in-discover-drilldown

e8aac1a

flash1293 added 2 commits May 3, 2022 11:28

fix open in new tab

d101a0d

fix open in new tab

9c48364

fix test

7225c50

make it possible to filter out drilldowns from list based on context

1854a21

flash1293 requested a review from a team as a code owner May 3, 2022 10:37

drewdaemon reviewed May 3, 2022

View reviewed changes

Merge remote-tracking branch 'upstream/main' into open-in-discover-dr…

4be7a1b

…illdown

review comments

718d779

drewdaemon approved these changes May 3, 2022

View reviewed changes

stratoula approved these changes May 4, 2022

View reviewed changes

vadimkibana mentioned this pull request May 5, 2022

Dashboard-to-XYZ drilldowns #61785

Closed

6 tasks

vadimkibana reviewed May 5, 2022

View reviewed changes

flash1293 added 3 commits May 5, 2022 13:51

Merge remote-tracking branch 'upstream/main' into open-in-discover-dr…

bceb68b

…illdown

Merge remote-tracking branch 'upstream/main' into open-in-discover-dr…

397d08a

…illdown

remove isConfigurable from the actionfactory

be542fd

Merge branch 'main' into open-in-discover-drilldown

ec535ea

vadimkibana approved these changes May 9, 2022

View reviewed changes

flash1293 merged commit a8017df into elastic:main May 9, 2022

KOTungseth mentioned this pull request May 16, 2022

[DOCS] [Lens] Make open in discover drilldown work #132248

Closed

Dosant mentioned this pull request May 31, 2022

Lens discover drilldown should use getHref and navigateToApp #133185

Closed

KOTungseth mentioned this pull request Jun 9, 2022

[DOCS] Adds the Vis Editor 8.3 docs #134115

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Lens] Make open in discover drilldown work #131237

[Lens] Make open in discover drilldown work #131237

flash1293 commented Apr 29, 2022 •

edited

Loading

elasticmachine commented May 2, 2022

stratoula commented May 3, 2022

stratoula commented May 3, 2022

flash1293 commented May 3, 2022

stratoula commented May 3, 2022

flash1293 commented May 3, 2022

flash1293 commented May 3, 2022

drewdaemon left a comment

stratoula left a comment

vadimkibana commented May 5, 2022

vadimkibana commented May 5, 2022

flash1293 commented May 5, 2022

vadimkibana May 5, 2022

vadimkibana May 5, 2022

flash1293 May 5, 2022

flash1293 commented May 6, 2022

kibana-ci commented May 9, 2022

API count

ESLint disabled line counts

References to deprecated APIs

Total ESLint disabled count

	actionFactory.isConfigurable(context)
	actionFactory.isCompatible(context)

[Lens] Make open in discover drilldown work #131237

[Lens] Make open in discover drilldown work #131237

Conversation

flash1293 commented Apr 29, 2022 • edited Loading

Tech details

Open question

elasticmachine commented May 2, 2022

stratoula commented May 3, 2022

stratoula commented May 3, 2022

flash1293 commented May 3, 2022

stratoula commented May 3, 2022

flash1293 commented May 3, 2022

flash1293 commented May 3, 2022

drewdaemon left a comment

Choose a reason for hiding this comment

stratoula left a comment

Choose a reason for hiding this comment

vadimkibana commented May 5, 2022

vadimkibana commented May 5, 2022

flash1293 commented May 5, 2022

vadimkibana May 5, 2022

Choose a reason for hiding this comment

vadimkibana May 5, 2022

Choose a reason for hiding this comment

flash1293 May 5, 2022

Choose a reason for hiding this comment

flash1293 commented May 6, 2022

kibana-ci commented May 9, 2022

💚 Build Succeeded

Metrics [docs]

Module Count

Public APIs missing comments

Async chunks

Page load bundle

API count

ESLint disabled line counts

References to deprecated APIs

Total ESLint disabled count

History

flash1293 commented Apr 29, 2022 •

edited

Loading