-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
"Explore underlying data" in-chart action #69494
"Explore underlying data" in-chart action #69494
Conversation
"explore data" actions use Discover URL generator, which is registered in "share" plugin, which is optional plugin, so we check for its existance, because otherwise URL generator is not available.
@elastic/kibana-design we preliminary agreed on above text ("Explore underlying data") and icon, but please comment if you think of better ones. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
KibanaApp owned Code LGTM, tested locally in Chrome, Firefox, Safari, works 👍
💚 Build SucceededBuild metrics@kbn/optimizer bundle module count
History
To update your PR or re-run it, just comment with: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, code review.
One thing up to you, we can discuss:
I am not sure about new explicit dependency embeddable -> data
because of trigger to filters helpers.
Because how many discussions we have today around this area and some changes are coming, I think I'd prefer it copy pasted for now.
x-pack/plugins/discover_enhanced/public/actions/explore_data/kibana_url.ts
Show resolved
Hide resolved
@Dosant I'll rename to @elastic/kibana-design If you would like to change icon or text please ping me. |
* master: (34 commits) Upgrade `elliptic` dependency (`6.5.2` → `6.5.3`). (elastic#70054) [License Management] Do not break when `telemetry.enabled:false` (elastic#69711) [SECURITY] Redirect app/security to app/security/overview (elastic#70005) "Explore underlying data" in-chart action (elastic#69494) Api reference docs for state_containers and state_sync (elastic#67354) prep state transfer for passing embeddables by value to editor and back (elastic#69991) move Metrics API to start (elastic#69787) refactor: 💡 fix typo in embeddable (elastic#69417) [alerting] migrates the old `alerting` consumer to be `alerts` (elastic#69982) [APM]Create API to return data to be used on the Overview page (elastic#69137) [Lens] Fix delete button position in dimension panel for long labels (elastic#69495) [Lens] Add toolbar api (elastic#69263) Fixes bug on color picker defaults on TSVB (elastic#69889) [DOCS] Fixes wording in Upload a CSV section (elastic#69969) [Discover] Validate timerange before submitting query to ES (elastic#69363) [Maps] avoid using MAP_SAVED_OBJECT_TYPE constant when defining URL paths (elastic#69723) [Maps] Fix icon palettes are not working (elastic#69937) [Ingest Manager] Fix typo in constant name (elastic#69919) [test] skip status.allowAnonymous tests on cloud (elastic#69017) Fix backport (elastic#70003) ...
* refactor: 💡 rename folder to "explore_data" * style: 💄 check for "share" plugin in more semantic way "explore data" actions use Discover URL generator, which is registered in "share" plugin, which is optional plugin, so we check for its existance, because otherwise URL generator is not available. * refactor: 💡 move KibanaURL to a separate file * feat: 🎸 add "Explore underlying data" in-chart action * fix: 🐛 fix imports after refactor * feat: 🎸 add start.filtersFromContext to embeddable plugin * feat: 🎸 add type checkers to data plugin * feat: 🎸 better handle empty filters in Discover URL generator * feat: 🎸 implement .getUrl() method of explore data in-chart act * feat: 🎸 add embeddable.filtersAndTimeRangeFromContext() * feat: 🎸 improve getUrl() method of explore data action * test: 💍 update test mock * fix possible stale hashHistory.location in discover * style: 💄 ensureHashHistoryLocation -> syncHistoryLocations * docs: ✏️ update autogenerated docs * test: 💍 add in-chart "Explore underlying data" unit tests * test: 💍 add in-chart "Explore underlying data" functional tests * test: 💍 clean-up custom time range after panel action tests * chore: 🤖 fix embeddable plugin mocks * chore: 🤖 fix another mock * test: 💍 add support for new action to pie chart service Co-authored-by: Anton Dosov <[email protected]> Co-authored-by: Anton Dosov <[email protected]>
Summary
Part of #69368
Adds "Explore underlying data" action to visualization value click and brush triggers.
Checklist
For maintainers