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

[wip] URL drilldown #68235

Closed
wants to merge 9 commits into from
Closed

[wip] URL drilldown #68235

wants to merge 9 commits into from

Conversation

Dosant
Copy link
Contributor

@Dosant Dosant commented Jun 4, 2020

Summary

URL Drilldown

issue
spec

Screenshot 2020-06-16 at 12 18 54


Reusability considerations:

This is not actually Url Drilldown, but SomeEmbeddablesToUrlDrilldown, as it relies on embeddable in drilldown context and on support of ValueClickTrigger & RangeSelectTrigger. This where url drilldown gets variables from from a tempate. More details here: spec

This EmbeddableToUrlDrilldown is registered in embeddable_enhanced plugin. But reusable building block to build other url Drilldowns are in uiActions_enhanced. How to use them showcased in SampleUrlDrilldown example.

@Dosant Dosant requested a review from streamich June 4, 2020 11:57
@Dosant Dosant requested a review from stacey-gammon June 4, 2020 11:59
@Dosant Dosant changed the title [wip] url drilldown [wip] First class license support in uiActions Jun 5, 2020
@Dosant Dosant changed the title [wip] First class license support in uiActions [wip] First class license support in uiActionsEnhanced Jun 5, 2020
@Dosant Dosant changed the title [wip] First class license support in uiActionsEnhanced [wip] URL drilldown Jun 8, 2020
@@ -0,0 +1,104 @@
/*
Copy link
Contributor

Choose a reason for hiding this comment

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

I would split the action license support into its own dedicated PR. To ensure the new APIs are used (and tested), you could modify one of the examples, or add a new one.

Copy link
Contributor Author

@Dosant Dosant Jun 9, 2020

Choose a reason for hiding this comment

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

@stacey-gammon, oh, forgot to remove reviewers from this pr. this is a cleaned up version of just license support: #68507

@Dosant Dosant removed the request for review from streamich June 9, 2020 12:35
@Dosant Dosant force-pushed the dev/url-drilldown branch from 7b69269 to 644fc50 Compare June 10, 2020 12:09
@Dosant Dosant requested review from streamich and ppisljar June 10, 2020 14:53
@mdefazio
Copy link
Contributor

I will put it on our list to create a description list UI for that variable popover

@streamich streamich mentioned this pull request Jun 17, 2020
16 tasks
const filtersFromEvent = await (async () => {
try {
if (isRangeSelectTriggerContext(context))
return await createFiltersFromRangeSelectAction(context.data);
Copy link
Member

Choose a reason for hiding this comment

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

as mentioned in the doc, i would prefer if we would not postprocess context.data and expose it directly. it will give more power to the user and it will be easier to extend url drilldown with future triggers.

…down

# Conflicts:
#	x-pack/plugins/ui_actions_enhanced/public/drilldowns/index.ts
@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Build metrics

@kbn/optimizer bundle module count

id value diff baseline
embeddableEnhanced 4 +1 3
uiActionsEnhanced 61 +37 24
total - +38 -

History

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants