-
Notifications
You must be signed in to change notification settings - Fork 8.5k
[Controls Anywhere] Fix panels being backed up when controls have unsaved changes #248029
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
base: main
Are you sure you want to change the base?
[Controls Anywhere] Fix panels being backed up when controls have unsaved changes #248029
Conversation
b04c2b8 to
51c4dc2
Compare
51c4dc2 to
a87d9f5
Compare
|
Pinging @elastic/kibana-presentation (Team:Presentation) |
| return true; | ||
| }; | ||
|
|
||
| export const areControlsLayoutsEqual = ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this method only talking about pinned controls? Should this be arePinnedPanelLayoutsEqual to better align with concept of pinned_panels and panels?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Summary
This PR ensures that only the necessary pieces of layout state are backed up when a dashboard has unsaved changes. Specifically, it fixes two separate bugs with backup state:
Consider the following code:
kibana/src/platform/plugins/shared/dashboard/public/dashboard_api/unsaved_changes_manager.ts
Lines 125 to 128 in 18f8010
Because [Controls Anywhere] Feature Branch #245588 made pinned controls normal children of the Dashboard rather than having separate unsaved changes management through the control group, the above code resulted in panels being backed up as unsaved changes even if only controls had changes. For example, making a single selection in a control resulted in both controls and panels being backed up to session storage - which caused extremely long URLs when
Short URLpermissions were turned off.Now consider the following:
kibana/src/platform/plugins/shared/dashboard/public/dashboard_api/unsaved_changes_manager.ts
Lines 83 to 88 in 18f8010
kibana/src/platform/plugins/shared/dashboard/public/dashboard_api/layout_manager/layout_manager.ts
Lines 492 to 517 in 18f8010
Because we were using
isLayoutEqual, which is checking for layout changes to both controls and panel layouts, we were reporting layout changes for panels even if only controls changed. For example, reordering a single control resulted in both controls and panels being backed up to session storage - which caused extremely long URLs whenShort URLpermissions were turned off.This PR fixes both of these bugs by...
We now compare which children actually changed and determine which ones are controls and which are normal panels. We only backup panels if at least one panel child changed, and same with controls.
We now separate out our comparison into
arePanelLayoutsEqualandareControlsLayoutsEqualand we conditionally return each piece of state as unsaved depending on the results.Checklist
release_note:*label is applied per the guidelinesbackport:*labels.