-
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
[Dashboard Navigation] Drill Down on Link Click #154381
Comments
Pinging @elastic/kibana-presentation (Team:Presentation) |
Noting here that when we navigate to different apps, we need to ensure that the current |
Noting here that we should also ensure that any state configured to be passed from one Dashboard to another should also apply when the link is opened in a new tab. |
Noting here that we will want to build some proper form validation around external URLs. See also this discussion. |
Closes #154381 ## Summary This PR adds functionality to the link embeddable so that 1) dashboard to dashboard links will now navigate between dashboards, taking the context (filters, time range, queries, etc.) as the user navigates (assuming the appropriate settings are applied) and 2) URL links will now navigate to internal and/or external URLs as expected ### Dashboard Links https://github.com/elastic/kibana/assets/8698078/1034b454-3add-48c2-8505-44d89e2d87d0 > **Note** > In the above video, all links were configured to include queries, filter pills, and the selected time range in the passed context. It is possible to disable these settings, if necessary. #### Link settings We want dashboard drilldowns and dashboard links to work more-or-less the same way, which means that they should have identical settings - therefore, in order to share the same settings between dashboard drilldowns + dashboard links, I created the `DashboardDrilldownOptionsComponent`: ![image](https://github.com/elastic/kibana/assets/8698078/5c809c7a-c9ef-43d7-b2ea-1d706858228a) I put this new component into `presentation_util` so that it could easily be shared and used in the respective editing experiences. ### Testing - When testing, special attention should be paid to the different link settings - ensure that all settings are respected. - Ensure that `ctrl + click`, `shift + click`, and `meta + click` work as expected, regardless of whether the `Open in new tab` setting is true or false - specifically, we should use the **URL** to pass the filters/queries/time range whenever a dashboard is being opened in a new tab. ### URL Links https://github.com/elastic/kibana/assets/8698078/5abf9772-17b3-4ab4-a704-4a3ff432fa23 #### Link settings Similar to the dashboard links, we want the ![image](https://github.com/elastic/kibana/assets/8698078/e9de2c81-fb2e-4d53-a610-7327466e4246) ### Testing When testing, it is important to ensure that the new URL validation works as expected. For this, there are technically two types of invalid links: 1) links that do not fit the expected format; for example, we do not accept any links that start with something other than `http`, `https`, or `mailto`. 2) links that are disallowed due to the `externalUrl.policy` Testing number one should be simple; however, in order to test the second type of invalid links, you need to modify your `kibana.dev.yml` to include something like the following: ``` externalUrl.policy: - allow: false host: danger.example.com - allow: true host: example.com protocol: https ``` Refer to http://www.elastic.co/guide/en/kibana/current/url-drilldown-settings-kb.html for more information. Also, much like with the dashboard links, it is important to ensure that `ctrl + click`, `shift + click`, and `meta + click` work as expected, regardless of whether the `Open in new tab` setting is true or false. ### Checklist - [x] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md) - [ ] ~[Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios~ Will be addressed in #161287 - [x] Any UI touched in this PR is usable by keyboard only (learn more about [keyboard accessibility](https://webaim.org/techniques/keyboard/)) - [x] Any UI touched in this PR does not create any new axe failures (run axe in browser: [FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/), [Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US)) - [x] This renders correctly on smaller devices using a responsive layout. (You can test this [in your browser](https://www.browserstack.com/guide/responsive-testing-on-local-server)) - [x] This was checked for [cross-browser compatibility](https://www.elastic.co/support/matrix#matrix_browsers) > **Note** > How Chrome and Firefox handle modified `onClick` events varies significantly and, through this testing, I found a bug where `shift + click`ing was previously not opening the links in a new tab in Firefox specifically - this is why I had to use the default to `href` behaviour whenever possible. Something to keep in mind when testing. ### For maintainers - [ ] This was checked for breaking API changes and was [labeled appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) --------- Co-authored-by: kibanamachine <[email protected]> Co-authored-by: Nick Peihl <[email protected]> Co-authored-by: Nick Peihl <[email protected]>
Fixed by #164196 |
Description
As part of the Dashboard Navigation project, we will need to fire off a drill-down to the destination dashboard when a dashboard link is clicked.
This issue is opened under the assumption that we will be able to use the existing Dashboard to Dashboard drilldown for this task, with some potential minor adjustments (not including a filter from the drill click for instance).
If this is not true, and we need to build a new drilldown object, we may need to revisit the LOE estimation.
The text was updated successfully, but these errors were encountered: