fix(mix): apply variants when resolving nested styles#926
Merged
Conversation
|
To view this pull requests documentation preview, visit the following URL: Documentation is deployed and generated using docs.page. |
Prop.resolveProp resolved a merged nested Style via resolve(), which only resolves props and skips variants. Context variants — widget states (hovered/pressed/disabled/focused), brightness, breakpoints, etc. — are applied by Style.build(), not Style.resolve(). So a Style nested inside another Style's Prop (a component sub-style) silently dropped its own variants. For example RemixSelectTriggerStyle().onHovered(...) had no effect because the trigger style is resolved as a nested prop of RemixSelectStyle. Build nested styles instead, so their variants resolve against the current context. This is a no-op for nested styles without variants (mergeActiveVariants returns the style unchanged), keeping the change scoped to styles that actually carry variants. Fixes btwld/remix#59 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
01e7bb1 to
26d1679
Compare
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
A
Stylestored inside anotherStyle'sProp— i.e. a component sub-style — silently drops its own variants.Prop.resolveProp()merges theMixSourcevalues and then resolves them withmergedMix.resolve(context). For aStyle,resolve()only resolves props; it does not apply variants. Context variants (widget stateshovered/pressed/disabled/focused,brightness, breakpoints, platform, …) are applied solely byStyle.build()(mergeActiveVariants(...)→resolve(...)). The top-level style goes throughStyleBuilder→style.build(context), but every nested style prop went throughresolve()— so its variants were quietly ignored.This is what makes
btwld/remix#59fail:The trigger style is stored as a nested
Prop<StyleSpec<RemixSelectTriggerSpec>>, so.onHovered/.onDisabledhad no effect. (Dropdown items worked only because each is built through its ownStyleBuilder+ controller.)Fix
Build nested styles instead of resolving them, so their context variants resolve against the current context:
mergeActiveVariantsreturns the style unchanged, then resolves identically to before. The blast radius is exactly "nested styles that carry variants."Stylehasbuild(); every otherMix(e.g.ModifierMix,DecorationMix,ShapeBorderMix) stays on the originalresolve()path via the type guard.mergedMixis aStyle<S>, the prop'sVisStyleSpec<S>, which is exactly whatbuild()returns.Verification
mixtest suite: 2716/2716 pass with the change.test/src/core/nested_style_variant_test.dartis red/green-proven (fails without the fix, passes with it).remixfull suite: 1742/1742 pass — issue Pressable freezes on press when onPressed: null #59 is fixed with no remix-side code change.Known boundary (follow-up, not addressed here)
StyleBuilderdecides whether to auto-install aWidgetStateProviderfrom the top-level style'swidgetStatesonly (it doesn't recurse into nested props). So a nested-only widget-state variant activates only when a state scope already exists (e.g. a component that passes acontroller, as remix'sSelectdoes). MakingStyle.widgetStatesrecurse into nested style props is a larger, separate change worth a dedicated issue.Fixes btwld/remix#59
🤖 Generated with Claude Code