Skip to content

fix(lit): add requestUpdate to TextField, DateTimeInput, CheckBox#1034

Open
ppazosp wants to merge 2 commits intogoogle:mainfrom
ppazosp:fix/1023-interactive-components-requestUpdate
Open

fix(lit): add requestUpdate to TextField, DateTimeInput, CheckBox#1034
ppazosp wants to merge 2 commits intogoogle:mainfrom
ppazosp:fix/1023-interactive-components-requestUpdate

Conversation

@ppazosp
Copy link
Copy Markdown

@ppazosp ppazosp commented Mar 30, 2026

Description

Refactors TextField, DateTimeInput, and CheckBox (v0.8) to use the updateBoundData() helper introduced in #1022, ensuring these components call requestUpdate() after setData().

Without this fix, these components update the data model but do not trigger a re-render, so other components bound to the same data path will not reflect the new value.

Fixes #1023
Depends on #1022

Pre-launch Checklist

If you need help, consider asking for advice on the discussion board.

ppazosp added 2 commits March 29, 2026 21:01
Add `updateBoundData()` helper to Root base class that encapsulates
the setData + requestUpdate pattern. Refactor Slider to use it.

Closes google#597
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request refactors data-binding logic across several UI components by introducing a centralized updateBoundData method in the Root base class, which reduces duplication and ensures consistent state updates. Feedback suggests further optimizing this by moving the path-existence guard clauses into the base method to simplify subclass implementations.

Comment on lines +79 to +90
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();
}
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.

medium

To further reduce code duplication, you could move the guard clauses from the subclasses into this helper method. This would make the updateBoundData method more robust and simplify the calling code in Checkbox, DateTimeInput, Slider, and TextField.

The signature could be changed to accept the object containing the path, rather than the path string itself.

For example:

protected updateBoundData(valueHolder: { path?: string } | null, value: DataValue) {
  if (!valueHolder || !('path' in valueHolder) || !valueHolder.path) {
    return;
  }

  if (!this.processor) {
    return;
  }

  this.processor.setData(
    this.component,
    valueHolder.path,
    value,
    this.surfaceId ?? A2uiMessageProcessor.DEFAULT_SURFACE_ID
  );
  this.requestUpdate();
}

Then, in the subclasses, the call would be simplified to this.updateBoundData(this.value, value); (or this.text) without any preceding guards.

  protected updateBoundData(valueHolder: { path?: string } | null, value: DataValue) {
    if (!valueHolder || !('path' in valueHolder) || !valueHolder.path) {
      return;
    }
    if (!this.processor) {
      return;
    }
    this.processor.setData(
      this.component,
      valueHolder.path,
      value,
      this.surfaceId ?? A2uiMessageProcessor.DEFAULT_SURFACE_ID
    );
    this.requestUpdate();
  }

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Good suggestion. Since updateBoundData() is defined in PR #1022 (not this PR), I will keep this PR focused on the three component fixes and address the signature change in #1022 if the maintainers agree with the approach.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Todo

Development

Successfully merging this pull request may close these issues.

bug(lit): v0.8 interactive components missing requestUpdate after setData

1 participant