[lucianlavric]

Fixes #21.

Root cause

CoreTextField passes only flexGrow={1} to its inner TextFieldWrapper, and Valdi's flexShrink defaults to 0. On web, the underlying valdi-input's intrinsic width is the HTML input's default (~200px at typical fonts), so the field cannot shrink below that and escapes narrower fixed-width parents — hidden horizontal page overflow even when the parent visually clips. The view model also exposed no way to size the container from the caller.

I verified the geometry in Chromium with a DOM reproduction of the rendered structure: with flex-shrink: 0 the input renders ~177px inside a 96px parent; with shrink allowed it fits at 62px.

Changes

• Pass flexShrink={1} and minWidth={0} to the inner TextFieldWrapper (it already forwards both) so the field shrinks below its intrinsic width when the parent is constrained.
• Add a containerStyle view model property, merged over the default container style via Style.merge, so callers can size the component root.
• Regression tests for both behaviors, watched failing before the fix.

Testing

All five CI targets pass locally (widgets, navigation, navigation_internal, playground, valdi_standalone_ui). I also verified the new tests fail when the fix is reverted.

Question

The new tests use valdiIt since createComponent is marked @deprecated, while the existing tests in this file still use createComponent. Is valdiIt the preferred direction for widgets tests going forward? Happy to migrate the rest of CoreTextFieldTest.ts here or in a follow-up if so.

[lucianlavric]

@clholgat thoughts?

[beaucollins]

Thanks for the contribution and the accompanying tests!

There's one concern around the performance of instantiating new Style objects during an onRender.

[beaucollins]

Creating or merging a Style during onRender() defeats the reuse optimization provided by Style.

See: https://github.com/Snapchat/Valdi/blob/main/docs/docs/core-styling.md#performance-considerations

Since the merged style depends only on viewModel.containerStyle, derive it in onViewModelUpdate() and retain it as a component instance property.

See: https://github.com/Snapchat/Valdi/blob/main/docs/docs/core-component.md#lifecycle

class CoreTextField extends StatefulComponent<CoreTextFieldViewModel, InternalState> {
  private mergedContainerStyle: Style<View> = styles.container;

  onViewModelUpdate(previousViewModel?: Readonly<CoreTextFieldViewModel>): void {
    const containerStyle = this.viewModel.containerStyle;

    if (
      previousViewModel &&
      previousViewModel.containerStyle === containerStyle
    ) {
      return;
    }

    this.mergedContainerStyle = containerStyle
      ? Style.merge(styles.container, containerStyle)
      : styles.container;
  }

  onRender(): void {
    <view style={this.mergedContainerStyle}>
      {/* ... */}
    </view>;
  }
}

This still pays the cost of constructing a new Style whenever containerStyle changes, but avoids paying it on every render. An unrelated view-model update with the same containerStyle retains the cached merged style. This assumes the caller also provides a stable containerStyle instance.

Also note that the directly supplied backgroundColor, borderWidth, and borderColor attributes take precedence over style. Values for those properties in containerStyle will therefore be ignored.

If that behavior is intentional, the API should communicate it by excluding those properties:

type ContainerStyle = Omit<
  View,
  'backgroundColor' | 'borderColor' | 'borderWidth'
>;

containerStyle?: Style<ContainerStyle>;

[lucianlavric]

Thanks for the review and the doc links! Both are addressed in eda22ee:

• The merge now happens in onViewModelUpdate() and is cached in mergedContainerStyle, so it's only rebuilt when the containerStyle instance changes, and onRender() reuses the same Style. I also added a note to the doc comment that callers should create the style once.
• Went with the Omit type, since those three are driven by the field's state: containerStyle is now Style<Omit<View, 'backgroundColor' | 'borderColor' | 'borderWidth'>>.

Separately, on the question in the description: should widgets tests use valdiIt going forward? Happy to migrate the rest of CoreTextFieldTest.ts in a follow-up :)

[beaucollins]

Thanks! I'll get this reviewed.

Re valdiIt, yes that's the correct helper to use. composerIt is deprecated due to a naming change.

[beaucollins]

Also worth noting this PR has me working through an API improvement that would allow for Style use that performs overrides on the native side.

So something like this:

<view style={[someStyle, someStyleOverride]}>
{/* ... */}
</view>

Which means you no longer would have to worry about state management and merging styles.

[lucianlavric]

That's sick! No more merging/caching in the component would definitely be easier... Happy to swap CoreTextField to style={[styles.container, viewModel.containerStyle]} once that's merged. I'd be interested in looking at the valdiIt test migration too if that's available in the future!