-
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] Better error handling for deleted dashboards #161563
Closed
Tracked by
#154354
Labels
Feature:Dashboard
Dashboard related features
impact:high
Addressing this issue will have a high level of impact on the quality/strength of our product.
loe:medium
Medium Level of Effort
Project:Dashboard Navigation
Related to the Dashboard Navigation Project
Team:Presentation
Presentation Team for Dashboard, Input Controls, and Canvas
Comments
nickpeihl
added
Team:Presentation
Presentation Team for Dashboard, Input Controls, and Canvas
loe:medium
Medium Level of Effort
impact:high
Addressing this issue will have a high level of impact on the quality/strength of our product.
Project:Dashboard Navigation
Related to the Dashboard Navigation Project
labels
Jul 10, 2023
Pinging @elastic/kibana-presentation (Team:Presentation) |
17 tasks
7 tasks
Heenawter
added a commit
that referenced
this issue
Aug 16, 2023
… error handling (#162285) Closes #154357 Closes #161563 ## Summary > **Warning** > I will be waiting to merge this PR until **after** #160896 is merged - I am simply opening it early so that we can start the design review process 👍 ### Layout This PR improves the rendering of the navigation embeddable to include both a horizontal and vertical layout option, as well as changing the style of how the links are rendered: https://github.com/elastic/kibana/assets/8698078/37d27683-a6c4-4e7a-9589-0eb0fb899e98 A known issue with the horizontal layout is that, as demonstrated in the above video, a "compact" horizontal navigation panel does not render as nicely in edit mode versus view mode - this is an **overall panel problem** and not specifically a problem with the navigation embeddable (although the navigation embeddable definitely makes it more obvious). This will be resolved for **all panels** by [removing the panel header altogether](#162182). ### Error handling This PR adds proper error handling to the navigation embeddable so that, if a dashboard link is "broken" (i.e. the destination dashboard has been deleted or cannot be fetched), an appropriate error message shows up in both the component and the editor flyout: https://github.com/elastic/kibana/assets/8698078/33a3e573-36a2-47ca-b367-3e04f9541ca3 > **Note** > When possible, we want to provide the user with as much context as possible for broken dashboard links - that is why, if a dashboard link was given a custom label, we still show this custom label even when the destination dashboard has been deleted/is unreachable. > > However, once a dashboard has been deleted, we no longer know what the title of that dashboard was because the saved object no longer exists - so, if a dashboard link is **not** given a custom label and the destination dashboard is deleted, we default to the "Error fetching dashboard" error message instead. In order to create a distinction between these two scenarios (a broken dashboard link with a custom label versus without), we italicize the generic "Error fetching dashboard" error text. ### Improved efficiency Previously, the navigation embeddable was handling its **own** dashboard cache, which meant that (a) every single embeddable had its own cache and (b) the navigation embeddable code had to be mindful when choosing to use the memoized/cached version of the dashboard versus fetching it fresh. After discussing with @ThomThomson about how to better handle this, we opted to move this logic to the dashboard content management service - not only does this clean up the navigation embeddable code, it also improves all the loading of dashboards in general. For example, consider the following video where I was testing re-loading a previously loaded dashboard on a throttled `Slow 3G` network: https://github.com/elastic/kibana/assets/8698078/41d68ac7-557c-4586-a59b-7268086991dd Notice in the above video how much faster the secondary load of the dashboard is in comparison to the first initial load - this is because, in the second load, we can hit the cache instead of re-fetching the dashboard from the content management client, which allows us to skip an entire loading state. ### 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) ### 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: Andrea Del Rio <delrio.andre@gmail.com> Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
Closed by #162285 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
Feature:Dashboard
Dashboard related features
impact:high
Addressing this issue will have a high level of impact on the quality/strength of our product.
loe:medium
Medium Level of Effort
Project:Dashboard Navigation
Related to the Dashboard Navigation Project
Team:Presentation
Presentation Team for Dashboard, Input Controls, and Canvas
If a Links panel includes a reference to a deleted Dashboard we only return an error in console and an empty link in the panel. We should return a user-facing error.
The text was updated successfully, but these errors were encountered: