Skip to content

Conversation

@Heenawter
Copy link
Contributor

@Heenawter Heenawter commented Jan 6, 2026

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:

  1. Consider the following code:

    if (hasChildrenUnsavedChanges) {
    dashboardBackupState.panels = panels;
    dashboardBackupState.controlGroupInput = controlGroupInput;
    }

    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 URL permissions were turned off.

  2. Now consider the following:

    const dashboardStateChanges$ = combineLatest([
    settingsManager.internalApi.startComparing$(lastSavedState$),
    unifiedSearchManager.internalApi.startComparing$(lastSavedState$),
    layoutManager.internalApi.startComparing$(lastSavedState$),
    projectRoutingManager?.internalApi.startComparing$(lastSavedState$) ?? of({}),
    ]).pipe(

    startComparing$: (
    lastSavedState$: BehaviorSubject<DashboardState>
    ): Observable<{ panels?: DashboardState['panels'] }> => {
    return layout$.pipe(
    debounceTime(100),
    combineLatestWith(
    lastSavedState$.pipe(
    map((lastSaved) =>
    deserializeLayout(lastSaved.panels, lastSaved.controlGroupInput, getReferences)
    ),
    tap(({ layout, childState }) => {
    lastSavedChildState = childState;
    lastSavedLayout = layout;
    })
    )
    ),
    map(([currentLayout]) => {
    if (!areLayoutsEqual(lastSavedLayout, currentLayout)) {
    logStateDiff('dashboard layout', lastSavedLayout, currentLayout);
    return serializeLayout(currentLayout, currentChildState);
    }
    return {};
    })
    );
    },

    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 when Short URL permissions were turned off.

This PR fixes both of these bugs by...

  1. 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.

  2. We now separate out our comparison into arePanelLayoutsEqual and areControlsLayoutsEqual and we conditionally return each piece of state as unsaved depending on the results.

Checklist

  • Unit or functional tests were updated or added to match the most common scenarios
  • The PR description includes the appropriate Release Notes section, and the correct release_note:* label is applied per the guidelines
  • Review the backport guidelines and apply applicable backport:* labels.

@Heenawter Heenawter added Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas t// loe:small Small Level of Effort release_note:skip Skip the PR/issue when compiling release notes impact:medium Addressing this issue will have a medium level of impact on the quality/strength of our product. labels Jan 6, 2026
@Heenawter Heenawter self-assigned this Jan 7, 2026
@Heenawter Heenawter force-pushed the dashboard-unsaved-changes-subset_2026-01-06 branch 2 times, most recently from b04c2b8 to 51c4dc2 Compare January 7, 2026 22:21
@Heenawter Heenawter force-pushed the dashboard-unsaved-changes-subset_2026-01-06 branch from 51c4dc2 to a87d9f5 Compare January 7, 2026 22:22
@Heenawter Heenawter changed the title [Dashboards] Store unsaved changes as a subset [Controls Anywhere] Fix panels being backed up when controls have unsaved changes Jan 7, 2026
@Heenawter Heenawter added the backport:skip This PR does not require backporting label Jan 8, 2026
@Heenawter Heenawter marked this pull request as ready for review January 8, 2026 18:00
@Heenawter Heenawter requested a review from a team as a code owner January 8, 2026 18:00
@elasticmachine
Copy link
Contributor

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

return true;
};

export const areControlsLayoutsEqual = (
Copy link
Contributor

@nreese nreese Jan 8, 2026

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?

Copy link
Contributor Author

@Heenawter Heenawter Jan 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are still kind of in an in-between state where we use controls some places and pinned_panels other places. But yeah, probably a good idea to just do this now - done in 04ac1dd :) We should be more consistent after #247958

@Heenawter Heenawter requested a review from nreese January 8, 2026 21:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport:skip This PR does not require backporting impact:medium Addressing this issue will have a medium level of impact on the quality/strength of our product. loe:small Small Level of Effort release_note:skip Skip the PR/issue when compiling release notes Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas t//

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants