-
Notifications
You must be signed in to change notification settings - Fork 8.2k
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
[Discuss][Meta] Dashboard By Value URL length considerations #71499
Comments
Pinging @elastic/kibana-app (Team:KibanaApp) |
Pinging @elastic/kibana-app-arch (Team:AppArch) |
I don't have an opinion yet on this, but a few things to take into consideration: Currently the behavior is also inconsistent because of this change, see #66875 Another con to consider is potential data loss - currently the unsaved dashboard state is part of the url, so navigating away from dashboard and back restores all unsaved changes. If these changes would only be kept in the dashboard state manager, they would be lost in that scenario. Lens (and Maps) are showing a modal when the user tries to navigate away from a page without having everything saved - that might be an option for Dashboard as well. In every way I would consider this a breaking change because you can't use dashboards in the same way as before anymore - we have a lot of users, there are definitely some which would be affected by this change. Another option we could consider: Saving "by reference" panels in the url, but keeping "by value" panels in the local state only (with a modal when trying to navigate away). "The old way" of doing things wouldn't be affected, and with the next major version we could switch the behavior for "by reference" panels over as well. The thing I'm not sure about is whether this effort is even necessary. |
Also don't have an opinion yet, but more considerations: We're currently relying on the URL handling to achieve undo when editing dashboards (and in other apps). This behavior is really trained by our users, and I am not sure if we can simply drop this behavior. If we would, we def need an alternative undo/redo solution in place. I also share the concern about losing state, that Joe mentioned, a lot. I think losing a dashboard state that's currently in edit mode, while doing a browser refresh is a no-go, even with warning it (or at least a significant breaking change). Maybe we should at least default to storing the state in localStorage. Related to the discussion in #35812 |
Totally agreed. See @stacey-gammon's comment about potentially removing url state in 8.0.0. Though I think it's also important to discuss what can be done to move us forward before then. Maybe a combination of both ideas could work in 7.x, where by reference panels show up in the url as before and any edits to by value panels are stored in localStorage, based on the This method would prevent data loss on refresh/navigation but would still break the ability to share edits to some embeddables on a dashboard via the url which could be frustratingly inconsistent. |
From my understanding this would leave the user with a very weird editing experience? Imagine a user would add a by value panel, then add a by reference panel, then add another by value panel, then resize the by value panel. If they now hit "back" in their browser, wouldn't suddenly just the reference panel be vanishing, and the two by value panels (added before and after it) staying around (in a potentially even broken grid configuration now, if the reference panel was the one holding those two together)? |
If I understood @ThomThomson correctly we should just use the existing URL state hashing functionality only for certain sub properties (the "by value" panels), while encoding everything else directly in the URL as it's working now.
|
What if we implemented "by value" embeddables with a feature flag and as part of that feature flag, removed all panel state from the URL? As long as users had the ability to the old URL state support, we should be able to switch it on by default in 7.x, with a warning that the having the flag off is deprecated and starting in 8.0, won't be supported anymore. Then we won't be in a situation of some state in URL, some not, but rather "new url state + by value support" or "old url state + no by value support". |
This strikes me as the best way forward. I am not sure about enabling the flag by default in 7.x, due to breaking changes concerns, but bundling makes the most sense. The two features Visualizations by value and fully descriptive urls are fundamentally at odds, and trying to mix them would definitely result in unnecessary complexity for the user, even if we avoid the type of situation that Tim described. |
My concern with not flipping it on by default is that you wouldn't get anyone trying out the "vis by value" feature, but we can always make that decision at a later date. |
occasionally users need to share the exact state they are looking at, that can be after they narrow the timerange and added a few filters. Will that still be possible? I got more than several complaints on the existing long URL, mainly since some systems don't allow users to paste the URL once it is above certain characters (zoom, confluent, few others). I agree with @timroes comment
Artificial way to reduce the likelihood of sharing a long URL : currently when clicking on share the default is a snapshot which is the unsaved version of the dashboard. In general, I think it should be the object (often people are can't figure why they don't see the dashboard latest updates) it will also help ensure the default sharing is using short URL. Again it is not a solution, more like an improvement Based on past experience with flags, users are not changing the default and it is not providing us any feedback to add new capability if the fly is off by default. Regarding breaking change, it is breaking change if the existing URLs users have doesn't work anymore or start showing something else |
@ThomThomson asked me to share some thoughts as an end user. Regarding using backward/forward nav as undo/redo buttons: I have never used it for this purpose, and always thought it was an UI oversight. I am on my laptop a lot while working dashboards and accidentally navigate away from my current version all the time (because of tracking pad movements) and it is super inconvenient; instead I try to save my dashboard periodically, or whenever I remember. Regarding verbose URL: we have a specific use case where we dynamically generate links to a dashboard based on different filter values; the verbose URL makes it harder to generate the URL, and also some systems don't allow us to embed URLs this long. Occasionally we save dashboards and close them, and then trying to re-open them by using browser auto-complete will bring up an older unsaved version of the dashboards; we'd have to go back to the Dashboard tab and try to find the object again; this is inconvenient to say the least. |
Note from today's meeting:
|
I just want to note that this enhancement is tracked by #8887, though it's ancient and maybe needs to be updated with the latest concerns if this is something we want to move forward with.
Losing form state by refreshing or navigating away from the form is a looming concern for ES UI. We manage many forms, some of which can be very complex, and losing state this way is painful. I'd be strongly in favor of a generalized solution to this problem that tracks form state in local storage and attempts to automatically repopulate it if the user navigates back and then forward again in browser history, refreshes the browser, or navigates away and then hits the back button. This tutorial demonstrates one method of solving this problem leveraging |
@flash1293 Good call, I am working on the Even with this approach though, I think the URL length considerations are still valid due to the fact each by value panel contains so much information that a dashboard with 200 by value panels could potentially exceed chrome's 2MB limit. Even though that is quite unlikely, I also consider the aesthetics of having such massive urls a deal breaker. If we move forward with removing panel state from the url, we will have to consider the user moving back and forth between apps while they edit. This necessitates a place to store the edited state while the user is gone. My current idea for the implementation is as follows:
The only feature this implementation is missing is undo-redo, but it could be added later via an in-memory solution as per #8887 Please let me know if you see any glaring issues in this implementation plan |
Related to: #84561
Problem
In the Time to Visualize project, embeddables can be stored by value inside dashboards. When a panel is stored by value, its state consists of all input required by the embeddable type instead of the panel's state consisting only of a saved object ID.
Currently, when a dashboard is in edit mode, the state of every panel is stored in the url, even the much larger
by value
panels.To illustrate the problem, here is a quick comparison of edit url length:
Doing a rough estimate / extrapolation it seems like a dashboard with 200 complicated by value panels could potentially exceed chrome's 2MB limit.
It's important to stress that crashing chrome like this is an unlikely edge case, therefore I wouldn't consider this a blocker for flipping the
allowByValueEmbeddables
switch. That said, releasing a project which balloons the url length without a plan to mitigate it doesn't seem like a great idea, not to mention the aesthetics of gigantic urls.A Solution
Recently, the state of dashboard panels in view mode was moved from the URL to state storage, which significantly cut down on the URL length in view mode.
I would suggest removing panels from the url in edit mode as well. Instead, the current state of the dashboards panels would be stored in sessionStorage, in a new object where the keys are dashboard Ids, and the values are listings of dashboard panels. When a panel is edited, removed or added, the sessionStorage is kept in sync. Saving would replace the panels in the saved object with the panels from session storage, and canceling the edit would do the reverse.
This would allow us to persist unsaved edits to multiple dashboards over reloads and navigations. If we go with this solution, we will have to take into account a number of considerations:
Considerations
Potential for Data Loss
There is a concern that removing the unsaved edits from the url could result in the loss of this data. This concern could be mitigated with the following changes:
OnAppLeave
handler will be used to ensure that the user doesn't leave dashboard with unsaved changes [Time to Visualize] Dashboard Warn on App Leave #81360Snapshot sharing
Currently, users are able to share unsaved edits to a dashboard without using the short url function. This lets users without write privileges share unsaved edits to dashboards. This functionality could be retained with the following implementation details:
Undo / Redo
Currently, panels in the url provide implicit undo / redo behaviour via the browser back and forward buttons. If they were removed from the url, we would have to provide an alternative means of achieving this such as memory based undo / redo #8887. This is an area with a big potential for improvement, since the current implementation has many problems that can be solved with this approach.
UX Improvements this could unlock
dashboard_state_manager
then standardized for use in other applications.ctrl + z
&ctrl + y
which are industry standardOpen Questions
The text was updated successfully, but these errors were encountered: