From 6ceb74d04da7c9902e34bdd5f2b263e557528c47 Mon Sep 17 00:00:00 2001 From: Leo Farias Date: Tue, 4 Aug 2026 19:42:27 -0400 Subject: [PATCH 1/2] fix(mix): discover widget-state dependencies through nested variants Style.widgetStates only scanned top-level WidgetStateVariants, so a style whose only widget-state variants were nested under another variant (for example onDark or onBreakpoint) or wrapped in NotVariant reported no dependencies. StyleBuilder then attached no MixInteractionDetector and those variants never activated. Collect dependencies recursively with identity-based cycle protection, and introduce ContextVariant.widgetStateDependencies so any context variant can declare the widget states it needs tracked. NotVariant delegates to its inner variant. Refs #967 (Style.widgetStates is one of its anchors; this addresses the under-tracking side, not the over-subscription concerns tracked there). --- packages/mix/CHANGELOG.md | 8 + packages/mix/lib/src/core/style.dart | 26 +++- packages/mix/lib/src/variants/variant.dart | 9 ++ .../src/core/style_builder_hover_test.dart | 138 ++++++++++++++++++ .../variants/widget_state_variant_test.dart | 20 +++ 5 files changed, 197 insertions(+), 4 deletions(-) diff --git a/packages/mix/CHANGELOG.md b/packages/mix/CHANGELOG.md index 82b5ea4bd3..1ec1175cce 100644 --- a/packages/mix/CHANGELOG.md +++ b/packages/mix/CHANGELOG.md @@ -1,3 +1,11 @@ +## Unreleased + +### Fixes + +- **Nested widget-state discovery:** `StyleBuilder` now discovers state + requirements recursively through nested and negated variants with + identity-based cycle protection. + ## 2.2.0-beta.1 ### New features diff --git a/packages/mix/lib/src/core/style.dart b/packages/mix/lib/src/core/style.dart index fd619c1a67..8e0e64251a 100644 --- a/packages/mix/lib/src/core/style.dart +++ b/packages/mix/lib/src/core/style.dart @@ -1,3 +1,5 @@ +import 'dart:collection'; + import 'package:flutter/foundation.dart'; import 'package:flutter/widgets.dart'; @@ -66,10 +68,26 @@ abstract class Style> extends Mix> @internal Set get widgetStates { - return ($variants ?? []) - .where((v) => v.variant is WidgetStateVariant) - .map((v) => (v.variant as WidgetStateVariant).state) - .toSet(); + final states = {}; + final visited = HashSet>.identity(); + + void collectDependencies(Style style) { + if (!visited.add(style)) return; + + final variants = style.$variants; + if (variants == null) return; + + for (final variantStyle in variants) { + if (variantStyle.variant case final ContextVariant variant) { + states.addAll(variant.widgetStateDependencies); + } + collectDependencies(variantStyle.value); + } + } + + collectDependencies(this); + + return states; } /// Merges all active variants with their nested variants recursively. diff --git a/packages/mix/lib/src/variants/variant.dart b/packages/mix/lib/src/variants/variant.dart index 67150f42b9..ed1e58b31d 100644 --- a/packages/mix/lib/src/variants/variant.dart +++ b/packages/mix/lib/src/variants/variant.dart @@ -108,6 +108,9 @@ class ContextVariant extends Variant { return ContextVariant.breakpoint(BreakpointToken.desktop()); } + /// Widget states that must be tracked for this variant to be evaluated. + Set get widgetStateDependencies => const {}; + /// Check if this variant should be active for the given context bool when(BuildContext context) { return shouldApply(context); @@ -185,6 +188,9 @@ final class NotVariant extends ContextVariant { bool operator ==(Object other) => identical(this, other) || other is NotVariant && other.inner == inner; + @override + Set get widgetStateDependencies => inner.widgetStateDependencies; + @override int get hashCode => inner.hashCode; } @@ -254,6 +260,9 @@ final class WidgetStateVariant extends ContextVariant { identical(this, other) || other is WidgetStateVariant && other.state == state; + @override + Set get widgetStateDependencies => {state}; + @override int get hashCode => state.hashCode; } diff --git a/packages/mix/test/src/core/style_builder_hover_test.dart b/packages/mix/test/src/core/style_builder_hover_test.dart index 983c9cd11f..09addfca20 100644 --- a/packages/mix/test/src/core/style_builder_hover_test.dart +++ b/packages/mix/test/src/core/style_builder_hover_test.dart @@ -5,6 +5,54 @@ import 'package:mix/mix.dart'; void main() { group('StyleBuilder hover functionality', () { + Future expectHoverColors( + WidgetTester tester, { + required BoxStyler style, + required Color initial, + required Color hovered, + Brightness platformBrightness = Brightness.light, + }) async { + Color? currentColor; + + await tester.pumpWidget( + MaterialApp( + home: MediaQuery( + data: MediaQueryData( + size: const Size(800, 600), + platformBrightness: platformBrightness, + ), + child: Center( + child: StyleBuilder( + style: style, + builder: (context, spec) { + currentColor = (spec.decoration as BoxDecoration?)?.color; + + return Container( + key: const Key('nested-hover-target'), + constraints: spec.constraints, + decoration: spec.decoration, + ); + }, + ), + ), + ), + ), + ); + + expect(currentColor, initial); + + final gesture = await tester.createGesture(kind: PointerDeviceKind.mouse); + await gesture.addPointer(location: Offset.zero); + await gesture.moveTo( + tester.getCenter(find.byKey(const Key('nested-hover-target'))), + ); + await tester.pump(); + + expect(currentColor, hovered); + + await gesture.removePointer(); + } + testWidgets('hover variant changes style when mouse enters and exits', ( tester, ) async { @@ -178,5 +226,95 @@ void main() { await gesture.removePointer(); }, ); + + group('nested widget-state dependency discovery', () { + testWidgets('tracks a negated hover variant', (tester) async { + final style = BoxStyler() + .size(100, 100) + .color(Colors.blue) + .onNot( + ContextVariant.widgetState(WidgetState.hovered), + BoxStyler().color(Colors.red), + ); + + await expectHoverColors( + tester, + style: style, + initial: Colors.red, + hovered: Colors.blue, + ); + }); + + testWidgets('tracks hover nested under a breakpoint', (tester) async { + final style = BoxStyler() + .size(100, 100) + .color(Colors.blue) + .onBreakpoint( + const Breakpoint.minWidth(0), + BoxStyler().onHovered(BoxStyler().color(Colors.red)), + ); + + await expectHoverColors( + tester, + style: style, + initial: Colors.blue, + hovered: Colors.red, + ); + }); + + testWidgets('tracks hover nested under dark mode', (tester) async { + final style = BoxStyler() + .size(100, 100) + .color(Colors.blue) + .onDark(BoxStyler().onHovered(BoxStyler().color(Colors.red))); + + await expectHoverColors( + tester, + style: style, + initial: Colors.blue, + hovered: Colors.red, + platformBrightness: Brightness.dark, + ); + }); + + testWidgets('tracks hover when breakpoint nesting is reversed', ( + tester, + ) async { + final style = BoxStyler() + .size(100, 100) + .color(Colors.blue) + .onHovered( + BoxStyler().onBreakpoint( + const Breakpoint.minWidth(0), + BoxStyler().color(Colors.red), + ), + ); + + await expectHoverColors( + tester, + style: style, + initial: Colors.blue, + hovered: Colors.red, + ); + }); + + test('handles cyclic nested variant styles by identity', () { + final variants = >[]; + final cyclicStyle = BoxStyler(variants: variants); + variants.add(VariantStyle(const NamedVariant('cycle'), cyclicStyle)); + variants.add( + VariantStyle( + ContextVariant.widgetState(WidgetState.hovered), + BoxStyler(), + ), + ); + final rootStyle = BoxStyler().variant( + const NamedVariant('root'), + cyclicStyle, + ); + + expect(rootStyle.widgetStates, {WidgetState.hovered}); + }); + }); }); } diff --git a/packages/mix/test/src/variants/widget_state_variant_test.dart b/packages/mix/test/src/variants/widget_state_variant_test.dart index fc87a0c5dd..3cfbef9953 100644 --- a/packages/mix/test/src/variants/widget_state_variant_test.dart +++ b/packages/mix/test/src/variants/widget_state_variant_test.dart @@ -89,6 +89,26 @@ void main() { final keys = variants.map((v) => v.key).toSet(); expect(keys.length, WidgetState.values.length); }); + + test('declares its widget state dependency', () { + final variant = ContextVariant.widgetState(WidgetState.hovered); + + expect(variant.widgetStateDependencies, {WidgetState.hovered}); + }); + + test('negated variants delegate widget state dependencies', () { + final variant = ContextVariant.not( + ContextVariant.widgetState(WidgetState.disabled), + ); + + expect(variant.widgetStateDependencies, {WidgetState.disabled}); + }); + + test('non-widget-state variants have no widget state dependencies', () { + final variant = ContextVariant.brightness(Brightness.dark); + + expect(variant.widgetStateDependencies, isEmpty); + }); }); group('Equality and hashCode', () { From 2a5743ad7104c638af0a982e53d30d113d8d45a0 Mon Sep 17 00:00:00 2001 From: Leo Farias Date: Thu, 6 Aug 2026 10:07:33 -0400 Subject: [PATCH 2/2] fix(mix): mount interaction detector only for pointer-driven states Discovering widget-state dependencies through nested variants made StyleBuilder mount MixInteractionDetector for states it cannot produce. The detector only derives hovered/pressed from pointer input; disabled, focused and the rest must come from an external controller or an ancestor scope, both of which already bypass it. Mounting it anyway had two visible costs: its opaque Listener swallowed pointer events aimed at widgets beneath it, and the state scope it opened was reused by descendants, so a nested box hovered on its ancestor's bounds instead of its own. Also: - declare the producible set on MixInteractionDetector so the fact lives with the code that implements it - drop the dart:collection import; Set.identity() is in dart:core - record why the visited set exists: cycles, and onBuilder storing the receiver as its placeholder, which is O(2^n) without dedup - document widgetStateDependencies as the override point for custom context variants, and its static-discovery limits - cover nested pressed, onEnabled discovery, hover exit, and both edges of the detector-mounting boundary --- packages/mix/CHANGELOG.md | 21 +- .../internal/mix_interaction_detector.dart | 13 + packages/mix/lib/src/core/style.dart | 12 +- packages/mix/lib/src/core/style_builder.dart | 17 +- packages/mix/lib/src/variants/variant.dart | 12 + .../src/core/style_builder_hover_test.dart | 242 +++++++++++++++++- 6 files changed, 305 insertions(+), 12 deletions(-) diff --git a/packages/mix/CHANGELOG.md b/packages/mix/CHANGELOG.md index 1ec1175cce..c3c269a347 100644 --- a/packages/mix/CHANGELOG.md +++ b/packages/mix/CHANGELOG.md @@ -1,10 +1,27 @@ ## Unreleased +### New features + +- **`ContextVariant.widgetStateDependencies`:** Context variants now declare the + widget states they read, so custom variants participate in nested dependency + discovery instead of relying on the framework recognizing a specific variant + type. Automatic self-tracking is limited to pointer-driven hover and press; + other states still require an ancestor scope or external controller. + ### Fixes -- **Nested widget-state discovery:** `StyleBuilder` now discovers state +- **Nested widget-state discovery:** `Style.widgetStates` now discovers state requirements recursively through nested and negated variants with - identity-based cycle protection. + identity-based cycle protection, so variants like + `onDark(BoxStyler().onHovered(...))` and `onEnabled(...)` are tracked instead + of silently never activating. +- **Interaction detector is mounted only when it can help:** `StyleBuilder` now + installs its pointer-interaction detector only for the states that detector + actually drives (`hovered`/`pressed`). States such as `disabled` and `focused` + can only come from an external `WidgetStatesController` or an ancestor scope, + so styles depending solely on those no longer gain an opaque hit-test target + that swallowed pointer events aimed at widgets beneath them, and no longer + hijack the state scope of descendants that do track hover. ## 2.2.0-beta.1 diff --git a/packages/mix/lib/src/core/internal/mix_interaction_detector.dart b/packages/mix/lib/src/core/internal/mix_interaction_detector.dart index 712fe81fdc..3ad3b6ef3d 100644 --- a/packages/mix/lib/src/core/internal/mix_interaction_detector.dart +++ b/packages/mix/lib/src/core/internal/mix_interaction_detector.dart @@ -23,6 +23,19 @@ class MixInteractionDetector extends StatefulWidget { this.onPointerPositionChange, }); + /// The widget states this detector derives from pointer input. + /// + /// Deliberately excludes [WidgetState.disabled]: that one is driven by + /// [enabled], which the caller sets, not by interaction. Every other + /// [WidgetState] must come from an external controller or an ancestor + /// [WidgetStateProvider], so installing this detector to satisfy them adds an + /// opaque hit-test target for no behavioural gain. Callers deciding whether + /// this detector is worth mounting should check against this set. + static const Set pointerDrivenStates = { + WidgetState.hovered, + WidgetState.pressed, + }; + final Widget child; final WidgetStatesController? controller; final bool enabled; diff --git a/packages/mix/lib/src/core/style.dart b/packages/mix/lib/src/core/style.dart index 8e0e64251a..7d046657d6 100644 --- a/packages/mix/lib/src/core/style.dart +++ b/packages/mix/lib/src/core/style.dart @@ -1,5 +1,3 @@ -import 'dart:collection'; - import 'package:flutter/foundation.dart'; import 'package:flutter/widgets.dart'; @@ -69,7 +67,15 @@ abstract class Style> extends Mix> @internal Set get widgetStates { final states = {}; - final visited = HashSet>.identity(); + // Identity, not equality, and it earns its keep twice over: + // 1. Cycles. `$variants` is stored by reference, so a caller can pass a + // list to a styler constructor and then append the styler to that same + // list. Value equality would also recurse forever comparing the cycle. + // 2. Sharing. `VariantStyleMixin.onBuilder` stores the receiver itself as + // the builder's placeholder value, so each chained `onBuilder` nests a + // snapshot of the style before it. Without dedup that is O(2^n) in the + // number of chained builders. + final visited = Set>.identity(); void collectDependencies(Style style) { if (!visited.add(style)) return; diff --git a/packages/mix/lib/src/core/style_builder.dart b/packages/mix/lib/src/core/style_builder.dart index ca5fab0eed..5fcc9e5592 100644 --- a/packages/mix/lib/src/core/style_builder.dart +++ b/packages/mix/lib/src/core/style_builder.dart @@ -125,11 +125,18 @@ class _StyleBuilderState> extends State> Widget build(BuildContext context) { final style = _buildStyle(context); - // Calculate interactivity need early - final needsToTrackWidgetState = - widget.controller == null && style.widgetStates.isNotEmpty; + // Calculate interactivity need early. Only states the detector can actually + // drive justify mounting it; see [MixInteractionDetector.pointerDrivenStates]. + final needsPointerStateTracking = + widget.controller == null && + style.widgetStates.any( + MixInteractionDetector.pointerDrivenStates.contains, + ); - final alreadyHasWidgetStateScope = WidgetStateProvider.of(context) != null; + // Variant resolution registers its own granular state dependencies; this + // existence check must not subscribe to every change in the model. + final alreadyHasWidgetStateScope = + context.getInheritedWidgetOfExactType() != null; Widget current = Builder( builder: (context) { @@ -142,7 +149,7 @@ class _StyleBuilderState> extends State> }, ); - if (needsToTrackWidgetState && !alreadyHasWidgetStateScope) { + if (needsPointerStateTracking && !alreadyHasWidgetStateScope) { // If we need interactivity and no MixWidgetStateModel is present, // wrap in MixInteractionDetector current = MixInteractionDetector(controller: _controller, child: current); diff --git a/packages/mix/lib/src/variants/variant.dart b/packages/mix/lib/src/variants/variant.dart index ed1e58b31d..a0c6ad17e4 100644 --- a/packages/mix/lib/src/variants/variant.dart +++ b/packages/mix/lib/src/variants/variant.dart @@ -109,6 +109,18 @@ class ContextVariant extends Variant { } /// Widget states that must be tracked for this variant to be evaluated. + /// + /// [Style.widgetStates] uses this declaration to discover dependencies in a + /// complete nested style. [StyleBuilder] can then install automatic tracking + /// for pointer-driven states such as hovered and pressed. Other states, such + /// as focused and disabled, still require an ancestor state scope or an + /// external [WidgetStatesController]. Subclasses that read widget state — + /// directly, or by delegating to another variant the way [NotVariant] does — + /// must override this getter. + /// + /// Discovery does not execute context closures, so states introduced by a + /// [ContextVariantBuilder], or read by a plain [ContextVariant] closure, + /// cannot contribute automatic dependencies. Set get widgetStateDependencies => const {}; /// Check if this variant should be active for the given context diff --git a/packages/mix/test/src/core/style_builder_hover_test.dart b/packages/mix/test/src/core/style_builder_hover_test.dart index 09addfca20..69315e0a67 100644 --- a/packages/mix/test/src/core/style_builder_hover_test.dart +++ b/packages/mix/test/src/core/style_builder_hover_test.dart @@ -39,7 +39,7 @@ void main() { ), ); - expect(currentColor, initial); + expect(currentColor, initial, reason: 'before hover'); final gesture = await tester.createGesture(kind: PointerDeviceKind.mouse); await gesture.addPointer(location: Offset.zero); @@ -48,7 +48,14 @@ void main() { ); await tester.pump(); - expect(currentColor, hovered); + expect(currentColor, hovered, reason: 'while hovered'); + + // Leaving must restore the original style too — a variant that latches on + // is just as broken as one that never activates. + await gesture.moveTo(Offset.zero); + await tester.pump(); + + expect(currentColor, initial, reason: 'after hover exit'); await gesture.removePointer(); } @@ -298,6 +305,59 @@ void main() { ); }); + testWidgets('tracks a nested pressed variant', (tester) async { + Color? currentColor; + + await tester.pumpWidget( + MaterialApp( + home: Center( + child: StyleBuilder( + style: BoxStyler() + .size(100, 100) + .color(Colors.blue) + .onBreakpoint( + const Breakpoint.minWidth(0), + BoxStyler().onPressed(BoxStyler().color(Colors.red)), + ), + builder: (context, spec) { + currentColor = (spec.decoration as BoxDecoration?)?.color; + + return Container( + key: const Key('nested-press-target'), + constraints: spec.constraints, + decoration: spec.decoration, + ); + }, + ), + ), + ), + ); + + expect(currentColor, Colors.blue, reason: 'before press'); + + final gesture = await tester.startGesture( + tester.getCenter(find.byKey(const Key('nested-press-target'))), + ); + await tester.pump(); + + expect(currentColor, Colors.red, reason: 'while pressed'); + + await gesture.up(); + await tester.pump(); + + expect(currentColor, Colors.blue, reason: 'after release'); + }); + + test('discovers the disabled dependency behind onEnabled', () { + // onEnabled is the public path that produces NotVariant(WidgetState), + // so it is the case users actually hit. + final style = BoxStyler() + .color(Colors.blue) + .onEnabled(BoxStyler().color(Colors.red)); + + expect(style.widgetStates, {WidgetState.disabled}); + }); + test('handles cyclic nested variant styles by identity', () { final variants = >[]; final cyclicStyle = BoxStyler(variants: variants); @@ -316,5 +376,183 @@ void main() { expect(rootStyle.widgetStates, {WidgetState.hovered}); }); }); + + group('interaction detector mounting', () { + // The detector wraps its child in an opaque Listener, so mounting it + // swallows pointer events that would otherwise fall through. It only + // earns that cost for states it can actually drive from pointer input. + Future reachesWidgetBeneath( + WidgetTester tester, + BoxStyler style, + ) async { + var tapped = false; + + await tester.pumpWidget( + MaterialApp( + home: Stack( + children: [ + Positioned.fill( + child: GestureDetector( + behavior: HitTestBehavior.opaque, + onTap: () => tapped = true, + child: const SizedBox.expand(), + ), + ), + Center( + child: Box( + key: const Key('overlay-box'), + style: style, + child: const SizedBox(), + ), + ), + ], + ), + ), + ); + + await tester.tapAt( + tester.getCenter(find.byKey(const Key('overlay-box'))), + ); + await tester.pump(); + + return tapped; + } + + // Every style below paints nothing, so the box itself never absorbs the + // hit and any difference comes from the detector alone. + testWidgets('a style with no widget states stays transparent to taps', ( + tester, + ) async { + expect( + await reachesWidgetBeneath(tester, BoxStyler().size(100, 100)), + isTrue, + ); + }); + + testWidgets('onEnabled alone stays transparent to taps', (tester) async { + // disabled can only come from a controller or an ancestor scope, both + // of which bypass the detector, so mounting it would be pure cost. + expect( + await reachesWidgetBeneath( + tester, + BoxStyler().size(100, 100).onEnabled(BoxStyler().size(100, 100)), + ), + isTrue, + ); + }); + + testWidgets('a nested hover variant does mount the detector', ( + tester, + ) async { + expect( + await reachesWidgetBeneath( + tester, + BoxStyler() + .size(100, 100) + .onBreakpoint( + const Breakpoint.minWidth(0), + BoxStyler().onHovered(BoxStyler().size(100, 100)), + ), + ), + isFalse, + ); + }); + + testWidgets('an ancestor does not hijack a descendant hover scope', ( + tester, + ) async { + // A descendant reuses an ancestor's state scope instead of opening its + // own, so an ancestor that mounts a detector it cannot use would make + // the descendant hover on the *ancestor's* bounds. + Color? innerColor; + + await tester.pumpWidget( + MaterialApp( + home: Center( + child: Box( + key: const Key('outer'), + style: BoxStyler() + .size(400, 400) + .onDisabled(BoxStyler().color(Colors.grey)), + child: Center( + child: StyleBuilder( + style: BoxStyler() + .size(50, 50) + .color(Colors.blue) + .onHovered(BoxStyler().color(Colors.red)), + builder: (context, spec) { + innerColor = (spec.decoration as BoxDecoration?)?.color; + + return Container( + key: const Key('inner'), + constraints: spec.constraints, + decoration: spec.decoration, + ); + }, + ), + ), + ), + ), + ), + ); + + final gesture = await tester.createGesture( + kind: PointerDeviceKind.mouse, + ); + await gesture.addPointer(location: Offset.zero); + + // Inside the outer box, far outside the inner one. + await gesture.moveTo( + tester.getCenter(find.byKey(const Key('outer'))) + + const Offset(150, 150), + ); + await tester.pump(); + + expect(innerColor, Colors.blue, reason: 'hovering the ancestor only'); + + await gesture.moveTo(tester.getCenter(find.byKey(const Key('inner')))); + await tester.pump(); + + expect(innerColor, Colors.red, reason: 'hovering the descendant'); + + await gesture.removePointer(); + }); + + testWidgets('scope lookup ignores unrelated state changes', ( + tester, + ) async { + final controller = WidgetStatesController(); + addTearDown(controller.dispose); + var builds = 0; + final child = StyleBuilder( + style: BoxStyler().size(50, 50), + builder: (context, spec) { + builds++; + + return Container(constraints: spec.constraints); + }, + ); + + await tester.pumpWidget( + MaterialApp( + home: ListenableBuilder( + listenable: controller, + builder: (context, _) => + WidgetStateProvider(states: controller.value, child: child), + ), + ), + ); + expect(builds, 1); + + controller.selected = true; + await tester.pump(); + + expect( + builds, + 1, + reason: 'checking for an ancestor scope must not subscribe to it', + ); + }); + }); }); }