Skip to content

[Dashboard] Fix setState in embeddables#260082

Merged
mbondyra merged 16 commits intoelastic:mainfrom
mbondyra:fix_state_for_children
Apr 1, 2026
Merged

[Dashboard] Fix setState in embeddables#260082
mbondyra merged 16 commits intoelastic:mainfrom
mbondyra:fix_state_for_children

Conversation

@mbondyra
Copy link
Copy Markdown
Contributor

@mbondyra mbondyra commented Mar 27, 2026

TL;DR:

The architecture changed from “children know how to reset themselves” to “parents own the target serialized state and tell children exactly which state to apply.”

To test

Please test your embeddables by:

  1. Making changes to the internals of your embeddable state (NOT grid changes)
  2. Verify "Unsaved changes" badge appears
  3. refresh the page - verify the state is as expected (with unsaved changes on)
  4. "reset changes" on Dashboard
  5. Verify the state has been properly reverted to the last saved state

Summary

I changed the embeddable state model so the framework no longer treats "reset to last saved" as the primitive operation. The primitive is now "apply this serialized state snapshot". This fixes the bug where dashboard-driven state updates were incorrectly using child reset behavior and therefore restoring the last saved child state instead of the state requested by the parent.

What Changed

The main architectural change is that child embeddables now expose an explicit serialized-state application capability through applySerializedState(state). Before this change, the dashboard reset/apply flow relied on resetUnsavedChanges(), which only means "go back to your own last saved baseline". That is the wrong abstraction when the parent already has a target state and wants the child to adopt that exact state.

Now the contract is:

  • Parent serializes and owns the desired dashboard state snapshot.
  • Layout manager deserializes child state from that snapshot.
  • Each child that supports serialized state application receives its exact next state through applySerializedState(...).
  • Dirty-state tracking remains separate from the question of what state should be applied.

Contract Changes

PublishesUnsavedChanges was simplified and now represents only:

  • serializeState()
  • applySerializedState(...)
  • hasUnsavedChanges$
    resetUnsavedChanges() was removed from the shared embeddable contract, because it was just convenience sugar and not the real framework primitive - so it's better not to have it, because otherwise we'll have to keep both in sync.

Dashboard / Container Flow

The dashboard container and layout manager were updated to use the new capability directly.
The flow is now:

  1. dashboardApi.setState(...) receives a full dashboard state snapshot.
  2. layoutManager deserializes layout and per-panel child state.
  3. For each existing child, if it supports serialized-state application, the layout manager calls child.applySerializedState(nextChildState).
  4. Children rehydrate themselves from the state provided by the parent.
    This is the key architectural fix: the container now pushes the intended state down, instead of asking children to infer it by resetting themselves.

Future improvements:

  1. Make serialized-state support more explicit in the type system. Right now PublishesUnsavedChanges implies state management capability; we could split “dirty tracking” from “serialized state manager” even more clearly if we want the contracts to stay very precise - but that's a bigger effort (a lot of reviewers too!) and I am happy to work later down the road.

for (const uuid of Object.keys(currentChildren)) {
if (layoutToApply.panels[uuid] || layoutToApply.pinnedPanels[uuid]) {
const child = currentChildren[uuid];
if (apiPublishesUnsavedChanges(child)) child.resetUnsavedChanges();
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

this line is a bug - instead of child.resetUnsavedChanges() we should set the state to the new state passed from the top.

@mbondyra mbondyra force-pushed the fix_state_for_children branch 2 times, most recently from 04ef3df to ce7ca20 Compare March 28, 2026 09:52
@mbondyra mbondyra marked this pull request as ready for review March 28, 2026 10:05
@mbondyra mbondyra requested review from a team as code owners March 28, 2026 10:05
@mbondyra mbondyra added Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas t// release_note:skip Skip the PR/issue when compiling release notes backport:skip This PR does not require backporting labels Mar 28, 2026
@elasticmachine
Copy link
Copy Markdown
Contributor

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

@mbondyra mbondyra force-pushed the fix_state_for_children branch 3 times, most recently from d1ba831 to 3d95df5 Compare March 29, 2026 18:08
@mbondyra mbondyra force-pushed the fix_state_for_children branch from 3d95df5 to 210852b Compare March 30, 2026 13:36
@botelastic botelastic bot added Feature:Drilldowns Embeddable panel Drilldowns Feature:Embedding Embedding content via iFrame labels Mar 30, 2026
@mbondyra mbondyra requested review from a team as code owners March 30, 2026 18:24
Copy link
Copy Markdown
Contributor

@ThomThomson ThomThomson left a comment

Choose a reason for hiding this comment

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

Posting partial review. A few changes are still needed. This is looking directionally correct, and I will finish my review before EOD today.

@@ -181,8 +181,8 @@ export const ControlGroupRenderer = ({
};
});
lastSavedState$Ref.current.next(newState);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: I don't think you need this anymore. updateInput used to be a hack that would set the last saved state to a value just in order to reset to it. Now you can applySerializedState to that new state directly.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Ok, noted — I’ve added a TODO to revisit this later.

As discussed asynchronously, my goal with this PR was to fix an existing bug and improve the structure, while avoiding triggering extensive cross-team reviews, since we need to merge it by tomorrow at the latest. This hasn't happen 😅 But as I mentioned, I’m happy to revisit it after FF.

For now, I’d prefer not to take on additional risk with this PR. I’m not sufficiently familiar with the controls to confidently sign off on this change without thorough testing, and I don’t have the time to do that properly at the moment, despite already working at more than full capacity.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

To me, putting off changing the code here actually increases the risk of this PR rather than decreasing it, as we'd be merging partial code without working through the full change set.

Judging by the tags, I'd assume you're aiming this for 9.4.0 feature freeze? If so, you have until April 7th. And this behaviour is fixing a bug anyway, so the feature freeze cutoff leaves some leeway.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I guess a 'nit' note has confused me. Thank you for explanation, tested and corrected.

@mbondyra mbondyra requested a review from ThomThomson March 30, 2026 19:13
@mbondyra mbondyra force-pushed the fix_state_for_children branch from ad2d25a to 213b5fa Compare March 30, 2026 19:17
@rbrtj rbrtj self-requested a review March 30, 2026 20:50
Copy link
Copy Markdown
Contributor

@rbrtj rbrtj left a comment

Choose a reason for hiding this comment

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

ML changes LGTM

@mbondyra mbondyra force-pushed the fix_state_for_children branch from 8cbd1f2 to d85cc39 Compare March 30, 2026 21:17
@mbondyra mbondyra requested a review from a team as a code owner March 30, 2026 21:17
@elastic elastic deleted a comment from elasticmachine Mar 30, 2026
Copy link
Copy Markdown
Contributor

@rmyz rmyz left a comment

Choose a reason for hiding this comment

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

obs presentation changes LGTM

@mbondyra mbondyra requested a review from a team as a code owner March 31, 2026 09:01
Copy link
Copy Markdown
Contributor

@stratoula stratoula left a comment

Choose a reason for hiding this comment

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

ES|QL team changes LGTM, I did a very brief testing in ES|QL controls and they work fine!

…e_for_children6

* commit '3402744f63ca1196e97b11ffac4e7f7efab240df': (80 commits)
  [PerUserAuth] Add EARS auth type for Connectors V2 (elastic#253695)
  Fix `@elastic/eui/require-aria-label-for-modals` lint violations across `@elastic/kibana-core` files (elastic#259757)
  [Entity Analytics][Leads generation][4] Add API routes, LeadDataClient, and async generation (elastic#257046)
  [Agent Builder] Agent-centric UX redesign (elastic#258005)
  fix query streams failing test (elastic#260277)
  [Lens as code] Add list layout to the new API (elastic#259967)
  [FTR] Add warning comments to deployment-agnostic FTR base configs (elastic#260018)
  [Discover][Logs profile] Fix missing search highlights (elastic#260056)
  Plugin system: safe deletion (elastic#259038)
  [Infra] Fix Hosts filter options to match selected schema (elastic#259825)
  Manual Entity Resolution and flyout representation (elastic#260162)
  [Cascade] Handle grouping on fields with unset values (elastic#260033)
  [Fleet] generate OTel config for integration packages with otelcol inputs (elastic#259968)
  [Search] Switch over to V2 index management details (elastic#259866)
  [inference] increase timeout for ES inference calls (elastic#260382)
  [ES|QL] Enable subqueries (elastic#257455)
  [ES|QL] Change Point order free options (elastic#260282)
  [Auth] Added authentication strategy for UIAM OAuth (elastic#256182)
  [Security Solution] Add "alerts_candidate_count" rule execution metric (elastic#259917)
  [api-docs] 2026-03-31 Daily api_docs build (elastic#260380)
  ...
Copy link
Copy Markdown
Contributor

@kdelemme kdelemme left a comment

Choose a reason for hiding this comment

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

LGTM

@elasticmachine
Copy link
Copy Markdown
Contributor

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] Investigations - Security Solution Cypress Tests #2 / Alert details expandable flyout rule preview panel alert reason preview "before each" hook for "should display alert reason preview" "before each" hook for "should display alert reason preview"

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
aiops 524.0KB 524.0KB -31.0B
apm 2.7MB 2.7MB -31.0B
canvas 1.0MB 1.0MB +20.0B
dashboard 847.6KB 847.6KB +35.0B
dashboardAgent 134.8KB 134.7KB -23.0B
dashboardMarkdown 73.3KB 73.3KB -31.0B
dataVisualizer 618.2KB 618.2KB -31.0B
discover 1.6MB 1.6MB -6.0B
embeddableAlertsTable 1.2MB 1.2MB +15.0B
imageEmbeddable 111.4KB 111.4KB -31.0B
infra 1.3MB 1.3MB +33.0B
lens 2.0MB 2.0MB +12.0B
links 103.5KB 103.5KB -31.0B
ml 5.8MB 5.8MB -31.0B
observability 2.2MB 2.2MB +33.0B
securitySolution 11.6MB 11.6MB +33.0B
slo 1.1MB 1.1MB +157.0B
synthetics 1.1MB 1.1MB -31.0B
triggersActionsUi 1.8MB 1.8MB +33.0B
visualizations 334.3KB 334.3KB -31.0B
total +63.0B
Unknown metric groups

API count

id before after diff
@kbn/presentation-publishing 451 452 +1

History

Copy link
Copy Markdown
Contributor

@ThomThomson ThomThomson left a comment

Choose a reason for hiding this comment

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

Changes LGTM! Thank you for applying the comments in the previous review, and everything is looking good now from a code standpoint.

I will write a followup PR to rename some of the arguments for more clarity, but we should get this one merged ASAP. Sorry for the review delay.

Also tested this locally and ensured resetting works as expected

@mbondyra mbondyra merged commit c23000f into elastic:main Apr 1, 2026
21 checks passed
@mbondyra mbondyra deleted the fix_state_for_children branch April 1, 2026 21:24
mbondyra added a commit to mbondyra/kibana that referenced this pull request Apr 1, 2026
…heck

* commit '6f040b29a5220ce12886a9731f656613e50aff06': (34 commits)
  [Entity Analytics] Add entity resolution UI to service flyout (elastic#260504)
  [Dashboard] Fix setState in embeddables (elastic#260082)
  [EDR Workflows] Unskip FTR tests that failed due to transient Fleet service unavailability (elastic#260519)
  [Observability:Streams] Fix query streams error handling test (elastic#260777)
  [Alerting v2] Dispatcher grouping modes, throttle strategies, and matcher autosuggestion (elastic#260249)
  [Dashboard] State extraction as a consistent override (elastic#259839)
  [Alerting v2] [Rule authoring] Fix rule name validation and error visibility in create/edit flow (elastic#260337)
  [Fix] re-introduce sln breadcrumbs to unified rules (elastic#260289)
  [Security Solution][Endpoint] Updated kibana docs to include `xpack.securitySolution.maxEndpointScriptFileSize` as configurable in cloud (elastic#260568)
  [Alerting v2] updated the alerting-v2-constants package with artifacts constants, fix to the runbook max characters (elastic#260342)
  [Automatic Import V2] Provide user tooltips (elastic#260725)
  [One Workflow] Deduplicate step types by base type in workflow list (elastic#260763)
  [Security Solution] Execution results UI: Enable the feature flag (elastic#260711)
  [Metrics][Discover] internal/search/esql_async returns 200 but METRICS_INFO responds with error (elastic#260746)
  Collapse redundant anyOf/oneOf array unions in OAS query params (elastic#260585)
  [Unified rules] Hide stack rules from global search (elastic#260088)
  [Agent Builder] Sidebar navigation updates (elastic#260728)
  [* As Code] Use PUT for upserts (elastic#260318)
  Update EUI to v114.0.0 (elastic#259497)
  [Entity Resolution] Add contextual-security-apps as co-owner of resolution paths (elastic#260659)
  ...

# Conflicts:
#	src/platform/plugins/shared/dashboard/public/index.ts
paulinashakirova pushed a commit to paulinashakirova/kibana that referenced this pull request Apr 2, 2026
#### TL;DR:
The architecture changed from “children know how to reset themselves” to
“parents own the target serialized state and tell children exactly which
state to apply.”

## To test
Please test your embeddables by:
1. Making changes to the internals of your embeddable state (NOT grid
changes)
2. Verify "Unsaved changes" badge appears
3. refresh the page - verify the state is as expected (with unsaved
changes on)
4. "reset changes" on Dashboard
5. Verify the state has been properly reverted to the last saved state

## Summary
I changed the embeddable state model so the framework no longer treats
"reset to last saved" as the primitive operation. The primitive is now
"apply this serialized state snapshot". This fixes the bug where
dashboard-driven state updates were incorrectly using child reset
behavior and therefore restoring the last saved child state instead of
the state requested by the parent.

## What Changed

The main architectural change is that child embeddables now expose an
explicit serialized-state application capability through
`applySerializedState(state)`. Before this change, the dashboard
reset/apply flow relied on `resetUnsavedChanges()`, which only means "go
back to your own last saved baseline". That is the wrong abstraction
when the parent already has a target state and wants the child to adopt
that exact state.

Now the contract is:
- Parent serializes and owns the desired dashboard state snapshot.
- Layout manager deserializes child state from that snapshot.
- Each child that supports serialized state application receives its
exact next state through `applySerializedState(...)`.
- Dirty-state tracking remains separate from the question of what state
should be applied.

## Contract Changes
`PublishesUnsavedChanges` was simplified and now represents only:
- `serializeState()`
- `applySerializedState(...)`
- `hasUnsavedChanges$`
`resetUnsavedChanges()` was removed from the shared embeddable contract,
because it was just convenience sugar and not the real framework
primitive - so it's better not to have it, because otherwise we'll have
to keep both in sync.

## Dashboard / Container Flow

The dashboard container and layout manager were updated to use the new
capability directly.
The flow is now:
1. `dashboardApi.setState(...)` receives a full dashboard state
snapshot.
2. `layoutManager` deserializes layout and per-panel child state.
6. For each existing child, if it supports serialized-state application,
the layout manager calls `child.applySerializedState(nextChildState)`.
7. Children rehydrate themselves from the state provided by the parent.
This is the key architectural fix: the container now pushes the intended
state down, instead of asking children to infer it by resetting
themselves.

## Future improvements:

1. Make serialized-state support more explicit in the type system. Right
now PublishesUnsavedChanges implies state management capability; we
could split “dirty tracking” from “serialized state manager” even more
clearly if we want the contracts to stay very precise - but that's a
bigger effort (a lot of reviewers too!) and I am happy to work later
down the road.

---------

Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
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 Feature:Drilldowns Embeddable panel Drilldowns Feature:Embedding Embedding content via iFrame release_note:skip Skip the PR/issue when compiling release notes Team:obs-presentation Focus: APM UI, Infra UI, Hosts UI, Universal Profiling, Obs Overview and left Navigation Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas t// v9.4.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.