fix: make ChoicePicker options bindable and DOM ids instance-unique - #27
Merged
Merged
Conversation
Narrows list-or-binding schemas per value so a literal list keeps resolving each element's own bindings while a bound list is watched whole, switches ChoicePicker's options to listOrReference, and adds ComponentScope.instanceId so ChoicePicker, Tabs and Modal no longer collide across surfaces or templated rows. Closes #18
CI's stable SDK (3.13.4) flags the conditional expression inside the list literal in narrowSchema, which the pinned 3.12.2 does not.
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
1 participant
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.
Summary
ChoicePicker'soptionscan now be bound to the data model or computed by a function call, and literal options still resolve each option's ownlabelbinding. Every component that derives a DOM id or radio group name from its component id now uses a key unique to the rendered instance, so two surfaces, or two rows of a template, no longer share a radio group or produce duplicate ids.Changes
A2uiComponentnarrows a list-or-binding property's schema to the alternative its current value takes before building the binder. A literal list resolves element by element, and a binding or call is watched as a whole. It rebinds when an update flips the value between the two.a2ui_coreclassifies each property once from its schema, so without this a literal list would arrive with its inner bindings unresolved.ChoicePicker.optionsis now the referencelistOrReferenceshape: a literal list, a data binding, or a function call. Bound entries that aren't maps are skipped instead of crashing the build.ComponentScope.instanceIdis built from the surface id, component id and data-model base path, with each part encoded so separators can't collide. ChoicePicker's radioname, the Tabs tab and panel ids, and Modal's dialog id all derive from it.Testing
The VM suite covers the binder contract directly.
test/rendering/list_or_reference_test.dartexercises literal, bound and flipping values, andtest/rendering/scope_test.dartcoversinstanceIdacross surfaces, templated rows and separator collisions. The Chrome test intest/browser/choice_picker_browser_test.dartis the one to read if you want to see the original bug: it renders two surfaces with the same picker id, clicks one, and checks the other's radio stays checked. It fails if you putscope.idback as the group name.Gates
Gated
de61343againstorigin/main@2d651c4.code-reviewonfable, passed, agent-reportedReviewed against the plan recorded on the issue.
Closes #18