elastic / kibana

Your window into the Elastic Stack
https://www.elastic.co/products/kibana
Other
19.79k stars 8.19k forks source link

[Serverless/Breadcrumbs] Path depending on origin #163488

Closed Dosant closed 8 months ago

Dosant commented 1 year ago

Context

In serverless navigation we changed how breadcrumbs work. Instead of setting the full path manually, we automatically calculate the main parts of the path from the side nav + current URL. This was done to keep side nav and breadcrumbs in sync as much as possible and solve consistency issues with breadcrumbs across apps. https://docs.elastic.dev/kibana-dev-docs/serverless-project-navigation#breadcrumbs

Issue with Lens/Maps/Visualize breadcrumbs

This works well for simple breadcrumbs, but in the non-serverless Kibana lens/visualize/maps have sophisticated breadcrumbs where context takes into account ByValue (if the edit is happening without a separate saved objected) and originatingApp and can switch depending on the context. For example, if the user is coming from "Dashboard" to edit "byValue" Lens visualization, Lens breadcrumbs display "Dashboard > Create", instead of "Visualization > Create".

In this PR we did a quick fix for the serverless breadcrumbs by simply appending the last ("title") part of the breadcrumb. But now we should bring back the original breadcrumbs functionality with changing Visualize <-> Dashboard context. We also need to figure out how to sync the changing context with the side nav, as we don't want to show "Dashboard" in the breadcrumb, but have "Visualization" highlighted in the side nav.

elasticmachine commented 1 year ago

Pinging @elastic/appex-sharedux (Team:SharedUX)

Dosant commented 1 year ago

Another similar use-case that is currently not possible with serverless breadcrumb: https://github.com/elastic/kibana/pull/163607/#pullrequestreview-1574271316

when clicking the root Discover breadcrumb from the single doc or surrounding docs view, the Discover context is lost (e.g. filters, query, saved search, etc.). It looks like the breadcrumb always points to /app/discover#/ which clears our URL state and causes the lost context.

Discover put the "return" state in "Discover" breadcrumb

vadimkibana commented 1 year ago

For example, if the user is coming from "Dashboard" to edit "byValue" Lens visualization, Lens breadcrumbs display "Dashboard > Create", instead of "Visualization > Create".

IMO, this sounds like the correct behaviour, if the active app is "Visualizations" then in the sidenav and breadcrumbs the "Visualizations" app should be active. If they still want to render "Dashboard" instead of "Visualizations" there, it should be possible also, they can disable the default behavior and add custom breadcrumbs: / Dashboard / <viz name>

vadimkibana commented 1 year ago

Another similar use-case that is currently not possible with serverless breadcrumb: https://github.com/elastic/kibana/pull/163607/#pullrequestreview-1574271316

when clicking the root Discover breadcrumb from the single doc or surrounding docs view, the Discover context is lost (e.g. filters, query, saved search, etc.). It looks like the breadcrumb always points to /app/discover#/ which clears our URL state and causes the lost context.

I think it should be possible with the Serverless breadcrumbs, they can disable the default breadcrumbs behavior and then render any breadcrumbs they want.

But I don't think that is the solution. I just checked the Discover app, it looks to be that reseting the URL when clicking on "Discover" breadcrumb is the correct behavior. The breadcumbs are above/outside of the context of discover filters, it makes perfect sense, IMO, that when one clicks on "Discover" breadcrumbs the app navigates to the landing page of Discover.

It sounds like what Discover could have is a "back" buttom which closes the single doc or surrounding doc views.

image

Or, the "correct way", IMO, if we want to do it through breadcrumbs would be to add the search context and document items in the breadcrumbs list:

image
sebelga commented 1 year ago

I agree with Vadim. User is in Dashboard. They click "Edit visualization". The fact that it is "ByValue" is an internal implementation details that the user should not know/care about. Not sure what is the problem to display "Visualization > Create" even for "ByValue" viz.

I think the teams have the API to customize everything if they really want:

Didn't test, but seems like it should be possible.

But like everything that looks so complex in the implementation, I must ask: why? 😊

vadimkibana commented 1 year ago

Also, would like to mention here something from a conversation in Slack, if the Visualize app wants to somehow show that the currently editing viz is coming from the Dashboard app, it should do something like:

/ Visualizations / Dashboard context / My Pie Chart

Note the "Dashboard context" breadcrumb item, which means the visualization was loaded from the by-value data passed from the Dashboard. If one clicks on the "Dashboard context" it would navigate back to the Dashboard app, to the place from where this editing context was started.

If one clicks on "Visualizations", it would go to the root of the Visualizations app, always, as expected. And "My Pie Chart" is not clickable, it just shows the title of the currently visible visualization.