fix(combobox): resolve trigger display text for pre-bound values in compositional mode#337
Open
mathewtaylor wants to merge 1 commit into
Open
fix(combobox): resolve trigger display text for pre-bound values in compositional mode#337mathewtaylor wants to merge 1 commit into
mathewtaylor wants to merge 1 commit into
Conversation
…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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
In compositional mode (
<BbComboboxItem>children, notOptions), the trigger resolves its caption from an internal text registry that's only populated when items mount. Items live insideBbPopoverContentwhich — since commit8ea4a8c4(the WASM perf fix that flippedForceMounttofalse) — does not mount until the popover first opens.The consequence: any
BbComboboxwith a pre-boundValuein 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 becauseRegisterItemwas a silent dictionary mutation — noStateHasChanged. 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
Optionscollection on every render.Fix
Three coordinated changes in
BbCombobox.razor.cs, no popover-default changes, perf fix preserved:1. Caller-provided
SelectedItemTextparameterCallers 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.
SelectedDisplayTextresolution chain (new step in bold)Options → registered items →
SelectedItemText→_selectedDisplayTextCache→ placeholderA registered item still wins over the caller hint so re-registration (e.g.
Textupdated by the parent) corrects stale captions.3.
RegisterItemtriggers a guarded re-renderThe new
_triggerTextDirtyflag is consulted byShouldRenderso the render isn't swallowed by the existing parameter-tracking guard. ThetextChangedcomparison is essential — without it theOnParametersSet-sideRegisterItemcall (which runs every parent cascade) would queue a render on every cycle.Behaviour matrix
Valuepre-bound, caller passesSelectedItemTextValuepre-bound, noSelectedItemTextTextupdated dynamicallyAlternatives considered
ForceMount="true"onBbCombobox's popover — fixes everything with no caller change, but reverts the8ea4a8c4perf win. Rejected for that reason.KeepItemsMountedopt-out parameter (defaulttrue, force-mount items) — same trade-off as above, with an escape hatch. Held in reserve if the "noSelectedItemTextprovided" case turns out to bite in practice.Func<TValue, string?> DisplayTextSelector— over-engineered; callers already have the string by the time they passValue.ChildContentoutside the popover — breaks portal isolation andBbCommandGrouplayout assumptions.Verification
- SelectedItemText : String).BbComboboxItem,BbPopover*, or any primitive — only one new optional parameter onBbCombobox.🤖 Generated with Claude Code