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

[Dashboard Navigation] Horizontal / Vertical Link Listing UI #154357

Closed
Tracked by #154354
ThomThomson opened this issue Apr 4, 2023 · 3 comments
Closed
Tracked by #154354

[Dashboard Navigation] Horizontal / Vertical Link Listing UI #154357

ThomThomson opened this issue Apr 4, 2023 · 3 comments
Assignees
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

@ThomThomson
Copy link
Contributor

Description

For the Dashboard Navigation project, we will need to extend the embeddable component to render a list of links in either a horizontal or vertical format. This should depend on the aspect ratio of the dashboard panel. In the Horizontal layout, these links should look similar to EUI Tabs and in the vertical layout they could look similar to the EUI Side nav.

@ThomThomson ThomThomson added Feature:Dashboard Dashboard related features Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas loe:medium Medium Level of Effort impact:medium Addressing this issue will have a medium level of impact on the quality/strength of our product. Project:Dashboard Navigation Related to the Dashboard Navigation Project labels Apr 4, 2023
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-presentation (Team:Presentation)

@Heenawter Heenawter self-assigned this May 15, 2023
@Heenawter Heenawter assigned Heenawter and unassigned Heenawter Jun 22, 2023
@Heenawter Heenawter removed their assignment Jul 5, 2023
@Heenawter Heenawter added impact:high Addressing this issue will have a high level of impact on the quality/strength of our product. and removed impact:medium Addressing this issue will have a medium level of impact on the quality/strength of our product. labels Jul 5, 2023
@Heenawter
Copy link
Contributor

After looking into how users currently use markdown panels for dashboard navigation, we have decided to instead make the horizontal/vertical layout a configurable setting per link panel rather than programmatically deciding the layout based on the aspect ratio of the panel.

This will give the users better flexibility for when, say, the title for their links is quite long (so the panel is longer than it is tall) but they still want a vertical layout.

@Heenawter Heenawter self-assigned this Jul 14, 2023
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 <[email protected]>
Co-authored-by: kibanamachine <[email protected]>
@Heenawter
Copy link
Contributor

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
Projects
None yet
Development

No branches or pull requests

3 participants