fix(lit): add updateBoundData helper and use it in interactive v0.8 widgets#1260
fix(lit): add updateBoundData helper and use it in interactive v0.8 widgets#1260ppazosp wants to merge 2 commits intogoogle:mainfrom
Conversation
Add `updateBoundData()` helper to Root base class that encapsulates the setData + requestUpdate pattern. Refactor Slider to use it. Closes google#597
There was a problem hiding this comment.
Code Review
This pull request refactors the Checkbox, DateTimeInput, Slider, and TextField components to utilize a new shared updateBoundData helper method in the Root class, streamlining data updates and ensuring consistent calls to requestUpdate(). Feedback suggests enhancing the updateBoundData method with a check for this.component to avoid unnecessary processing and redundant updates when the component is not fully initialized.
| protected updateBoundData(relativePath: string, value: DataValue) { | ||
| if (!this.processor) { | ||
| return; | ||
| } | ||
| this.processor.setData( | ||
| this.component, | ||
| relativePath, | ||
| value, | ||
| this.surfaceId ?? A2uiMessageProcessor.DEFAULT_SURFACE_ID | ||
| ); | ||
| this.requestUpdate(); | ||
| } |
There was a problem hiding this comment.
The updateBoundData helper should verify that this.component is not null before proceeding. While processor.setData includes an internal check and logs a warning, explicitly checking it here prevents an unnecessary call and a redundant requestUpdate() when the component context is missing. This ensures the helper only executes when the component is fully initialized and attached to a node.
| protected updateBoundData(relativePath: string, value: DataValue) { | |
| if (!this.processor) { | |
| return; | |
| } | |
| this.processor.setData( | |
| this.component, | |
| relativePath, | |
| value, | |
| this.surfaceId ?? A2uiMessageProcessor.DEFAULT_SURFACE_ID | |
| ); | |
| this.requestUpdate(); | |
| } | |
| protected updateBoundData(relativePath: string, value: DataValue) { | |
| if (!this.processor || !this.component) { | |
| return; | |
| } | |
| this.processor.setData( | |
| this.component, | |
| relativePath, | |
| value, | |
| this.surfaceId ?? A2uiMessageProcessor.DEFAULT_SURFACE_ID | |
| ); | |
| this.requestUpdate(); | |
| } |
Description
Adds a
updateBoundData()helper to theRootbase class that encapsulates thesetData()+requestUpdate()pattern, and wires up all interactive v0.8 widgets (Slider,TextField,DateTimeInput,CheckBox) to use it.Root cause: these components call
processor.setData()to update the data model, but neverrequestUpdate(), so the signal change fromSignalMap.set()is not picked up bySignalWatcherandrender()is never called again. Other components bound to the same data path will not reflect the new value. Visible inSlider(the<span>showing the current value stays stuck), latent in the others because native<input>elements display typed characters regardless of Lit re-renders.Fix: Centralize the
setData()+requestUpdate()pattern inRoot.updateBoundData()and migrate all four components to use it. Consistent with the existing pattern inMultipleChoice(multiple-choice.ts:308).Also removes an unused
Typesimport from the Slider.Closes #597
Fixes #1023
Pre-launch Checklist