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

Drilldowns in examples #75640

Merged
merged 75 commits into from
Oct 5, 2020
Merged

Drilldowns in examples #75640

merged 75 commits into from
Oct 5, 2020

Conversation

streamich
Copy link
Contributor

@streamich streamich commented Aug 21, 2020

Summary

Closes #58027

This PR adds developer examples of Drilldown Manager usage outside of Dashboard app.

yarn start --run-examples

go to "Developer examples" -> "UI Actions Enhanced" example plugin.

image

  • Refactors Dashboard-to-Dashboard drilldown such that it can be used in other places.
  • Adds example plugin for ui_actions_enhanced plugin.
  • "Without embeddable example" shows how Drilldown Manager could be used in a setting without embeddables and very lean trigger context, such is ML app use case.
  • "Without embeddable example (single button)" shows how the same button could be reused for Drilldown Manager actions as well as drilldown dynamic actions, could be useful for Canvas, if they decide to add triggers to elements.
  • "With embeddable example" shows that URL drilldown could be re-used in other apps if they use embeddables and VALUE_CLICK_TRIGGER.

@streamich
Copy link
Contributor Author

@elasticmachine merge upstream

Copy link
Contributor

@Dosant Dosant left a comment

Choose a reason for hiding this comment

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

Looks good,

  1. I think removing ml / canvas reference in code is a right call, I just think we should finalise it as there are still references, for example, in trigger contexts shapes.

  2. The example app is so nice. I'd just think we should push descriptions / displayNames / labels a bit further. I don't think anyone would get an idea that apps can register their own triggers and create there own drilldown types for those triggers by just playing with the app. For example: it is not clear that we have different "go to dashboard" implementations from example app.

  3. Minor nit, let's at least add description?

Screenshot 2020-09-30 at 12 25 23

Some other asks below. Some of them intersect with points above

@streamich
Copy link
Contributor Author

@Dosant

  1. I've removed references, except the very deep ones inside the context object, which are not really references to ML or Canvas but just shows that the context can have app specific data.

  2. Yes, I think for that we need to add explanations to the README, I've added explanation of what is contained in the sample folders.

  3. I've added the description.

@streamich streamich requested a review from Dosant September 30, 2020 17:58
@spalger spalger removed the request for review from a team September 30, 2020 19:34
@streamich
Copy link
Contributor Author

@elasticmachine merge upstream

Copy link
Contributor

@Dosant Dosant left a comment

Choose a reason for hiding this comment

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

LGTM, didn't retest latest changes.
One thing: https://github.com/elastic/kibana/pull/75640/files#r498278987 (note about dashboard word in toast)
Or I guess it is fine to change in scope of #78997

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

@kbn/optimizer bundle module count

id before after diff
dashboardEnhanced 26 28 +2
discoverEnhanced 11 10 -1
share 38 39 +1
uiActionsEnhanced 134 138 +4
total +6

page load bundle size

id before after diff
dashboard 570.2KB 570.3KB +60.0B
dashboardEnhanced 43.1KB 49.5KB +6.4KB
discoverEnhanced 29.2KB 27.1KB -2.1KB
share 79.9KB 82.1KB +2.2KB
uiActionsEnhanced 320.7KB 326.8KB +6.1KB
total +12.6KB

History

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

Copy link
Contributor

@majagrubic majagrubic left a comment

Choose a reason for hiding this comment

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

Had a look at the code, didn't run the examples. kibana-app code looks good to me 👍
Just a few nits I noticed below 👇

@streamich streamich merged commit 59e4e06 into elastic:master Oct 5, 2020
@streamich streamich added v7.10.0 and removed v7.11.0 labels Oct 5, 2020
jloleysens added a commit to jloleysens/kibana that referenced this pull request Oct 5, 2020
…nes/fix-description-field

* 'master' of github.com:elastic/kibana:
  A11y tests for user page (elastic#79199)
  [Ingest Pipelines] Processors editor a11y focus states (elastic#79122)
  [Ingest pipelines] Clean up component integration tests (elastic#78838)
  Drilldowns in examples (elastic#75640)
  Storybook and Jest cleanup (elastic#79305)
  adds EQL sequence rule test (elastic#79287)
  PR template a11y checklist item improvement (elastic#79243)
  [Security Solution] Adding tests for dns pipeline in the endpoint package (elastic#79177)
  [ML] Only adjust the bounds of SMV if annotations are visible (elastic#79210)
  global search to ts refs (elastic#79446)
  [Index management] Update TemplateDeserialized interface (elastic#78913)
  [Telemetry] server fetcher check all collectors ready before sending (elastic#79398)
  [Mappings editor] Fix app crash when selecting "other" field type (elastic#79434)
  [`/api/stats`] Add documentation + small improvement (elastic#79330)
  [Discover] "View surrounding documents" encodes spaces in filters (elastic#79283)
  [Lens] refactor DimensionContainer and fix flyout bug (elastic#79277)

# Conflicts:
#	x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/components/pipeline_processors_editor_item/inline_text_input.tsx
#	x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/components/processors_tree/components/private_tree.tsx
streamich added a commit that referenced this pull request Oct 6, 2020
* feat: 🎸 add telemetry for in-chart "Explore underlying data"

* feat: 🎸 add telemetry for in-chart "Explore underlying data"

* refactor: 💡 move all drilldowns into a sub-folder

* feat: 🎸 setup example app section for ui_actions_enhanced

* feat: 🎸 set up Drilldown Manager section

* feat: 🎸 open drilldown manager from example plugin

* refactor: 💡 rename supportedTriggers -> triggers prop

* feat: 🎸 show dev warning if triggers prop is empty

* refactor: 💡 rename "supportedTriggers" -> "triggers" props

* feat: 🎸 open and close drilldown manager from example plugin

* feat: 🎸 add sample ML job trigger

* feat: 🎸 add sample ML URL drilldown

* refactor: 💡 move KibanaURL to share plugin

* refactor: 💡 add index file to ml drilldown

* feat: 🎸 add AbstractDashboardDrilldown

* refactor: 💡 make dashboard drilldown use abstract drilldown

* refactor: 💡 rename dashboard drilldown to embeddable drilldown

* feat: 🎸 add Dashboard drilldown to sample plugin

* feat: 🎸 open dashboard drilldown in list view

* feat: 🎸 add drilldown execute button

* refactor: 💡 move drilldown React hooks into /hooks folder

* test: 💍 fix tests after renaming triggers

* chore: 🤖 populate "requireBundles"

* fix: 🐛 fix TypeScript errors

* fix: 🐛 fix Kibana plugin dependency

* chore: 🤖 remoe unused import

* feat: 🎸 persist drilldown manager state across app navigations

* refactor: 💡 move no-embeddable example into a seprate file

* feat: 🎸 set up example with embeddable

* feat: 🎸 improve embeddable example

* refactor: 💡 rename without embeddable example

* feat: 🎸 set up no-embeddable single click example

* feat: 🎸 add dashboard drilldown to single button example

* fix: 🐛 remove unused margin

* fix: 🐛 make "Get more actions" translation static

* chore: 🤖 remove old dashboard drilldown definition

* refactor: 💡 rename samples to generic names

* refactor: 💡 make app 1 example drilldown "hello world"

* chore: 🤖 remove unused required bundle

* chore: 🤖 add dashboardEnhanced back

* [kbn/optimizer] only build xpack examples when building xpack plugins

* move alerting_example into x-pack/examples

* remove filter for alertingExample plugin in oss plugins CI step

* revert unrelated change

* fix: 🐛 use correct prop name

* test: 💍 fix embeddable-to-dashboard drilldown mock

* test: 💍 fix a test after refactor

* chore: 🤖 remove unused import

* chore: 🤖 add dashboard_enahcned to example plugin

* chore: 🤖 address review comments

* feat: 🎸 add description to UI Actions Enhanced examples

* docs: ✏️ improve docs of example plugin

Co-authored-by: spalger <[email protected]>
Co-authored-by: Elastic Machine <[email protected]>
Co-authored-by: Kibana Machine <[email protected]>

Co-authored-by: spalger <[email protected]>
Co-authored-by: Elastic Machine <[email protected]>
Co-authored-by: Kibana Machine <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Drilldowns Embeddable panel Drilldowns Feature:UIActions UI actions. These are client side only, not related to the server side actions.. release_note:skip Skip the PR/issue when compiling release notes v7.10.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Drilldown demos
7 participants