Skip to content

fix(combobox): resolve trigger display text for pre-bound values in compositional mode#337

Open
mathewtaylor wants to merge 1 commit into
developfrom
fix/combobox-trigger-display-text
Open

fix(combobox): resolve trigger display text for pre-bound values in compositional mode#337
mathewtaylor wants to merge 1 commit into
developfrom
fix/combobox-trigger-display-text

Conversation

@mathewtaylor
Copy link
Copy Markdown
Contributor

Problem

In compositional mode (<BbComboboxItem> children, not Options), the trigger resolves its caption from an internal text registry that's only populated when items mount. Items live inside BbPopoverContent which — since commit 8ea4a8c4 (the WASM perf fix that flipped ForceMount to false) — does not mount until the popover first opens.

The consequence: any BbCombobox with a pre-bound Value in compositional mode renders the placeholder instead of the selected item's text on initial paint. Worse, even after the popover opens once and items register, the trigger still doesn't update because RegisterItem was a silent dictionary mutation — no StateHasChanged. The placeholder only got replaced when something else triggered a render (e.g. the user actually selected an item).

Options mode is immune: it resolves text synchronously against the Options collection on every render.

Fix

Three coordinated changes in BbCombobox.razor.cs, no popover-default changes, perf fix preserved:

1. Caller-provided SelectedItemText parameter

[Parameter]
public string? SelectedItemText { get; set; }

Callers that already know the display text for a pre-bound value (typical: the value came from an API alongside its label) can now render it on initial paint without waiting for items to mount.

2. SelectedDisplayText resolution chain (new step in bold)

Options → registered items → SelectedItemText_selectedDisplayTextCache → placeholder

A registered item still wins over the caller hint so re-registration (e.g. Text updated by the parent) corrects stale captions.

3. RegisterItem triggers a guarded re-render

internal void RegisterItem(TValue value, string text)
{
    if (value is null) return;

    var textChanged = !_itemTextRegistry.TryGetValue(value, out var existing)
        || !string.Equals(existing, text, StringComparison.Ordinal);

    _itemTextRegistry[value] = text;

    if (textChanged && EqualityComparer<TValue>.Default.Equals(value, Value))
    {
        _triggerTextDirty = true;
        StateHasChanged();
    }
}

The new _triggerTextDirty flag is consulted by ShouldRender so the render isn't swallowed by the existing parameter-tracking guard. The textChanged comparison is essential — without it the OnParametersSet-side RegisterItem call (which runs every parent cascade) would queue a render on every cycle.

Behaviour matrix

Scenario Before After
Compositional, Value pre-bound, caller passes SelectedItemText Placeholder forever Correct text on initial paint
Compositional, Value pre-bound, no SelectedItemText Placeholder forever Placeholder until first popover open, then snaps to correct text
Compositional, item Text updated dynamically Trigger stuck on first-mount caption Trigger updates on next re-registration
Options mode ✓ (no change)

Alternatives considered

  • ForceMount="true" on BbCombobox's popover — fixes everything with no caller change, but reverts the 8ea4a8c4 perf win. Rejected for that reason.
  • KeepItemsMounted opt-out parameter (default true, force-mount items) — same trade-off as above, with an escape hatch. Held in reserve if the "no SelectedItemText provided" case turns out to bite in practice.
  • Func<TValue, string?> DisplayTextSelector — over-engineered; callers already have the string by the time they pass Value.
  • Pre-render ChildContent outside the popover — breaks portal isolation and BbCommandGroup layout assumptions.

Verification

  • Components build clean.
  • 67/67 API surface snapshot tests pass after baselining the new parameter (single-line addition: - SelectedItemText : String).
  • No changes to BbComboboxItem, BbPopover*, or any primitive — only one new optional parameter on BbCombobox.

🤖 Generated with Claude Code

…ompositional mode

In compositional mode (BbComboboxItem children, not Options) the trigger
resolves its caption from an internal registry that's populated only when
the items mount. Items live inside BbPopoverContent which doesn't mount
until the popover first opens, so a pre-bound Value rendered placeholder
until the user interacted with the dropdown. After commit 8ea4a8c
flipped BbPopoverContent.ForceMount to false (a WASM perf win) this
became the steady-state behaviour rather than a transient.

This change closes the gap without undoing the perf fix:

- Adds SelectedItemText parameter so callers that already know the
  display text for a pre-bound value (typical when the value comes from
  an API alongside its label) can render it on initial paint. Slotted
  in SelectedDisplayText after the registry so re-registration still
  corrects stale captions.

- RegisterItem now marks a _triggerTextDirty flag and calls
  StateHasChanged when a freshly-registered item matches the current
  Value AND its text actually changed. ShouldRender treats the flag as
  a render trigger. Without this, items registering on first popover
  open did not cause the trigger to re-evaluate, so the placeholder
  stuck even after the registry was populated.

- textChanged guard avoids spurious renders from the OnParametersSet-
  side RegisterItem call that re-registers identical text every parent
  cascade.

Options mode is unaffected (it resolves synchronously from the Options
collection and never hit this issue). BbPopoverContent.ForceMount stays
false; BbComboboxItem is unchanged.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant