From 2694fcaee2bb150df9163bb536d70937d99117be Mon Sep 17 00:00:00 2001 From: Leo Farias Date: Sat, 11 Jul 2026 10:40:55 -0400 Subject: [PATCH 1/4] fix(mix): make style resolution reactive and deterministic - Make Style.of/maybeOf register an inherited-widget dependency so a StyleProvider change re-resolves dependent StyleBuilders even when the child widget instance is identical. - Replace the all-aspects WidgetStateProvider.of existence check with a read-only hasProvider, so state-independent styles no longer re-resolve when unrelated hover/press/focus states change above them. - Drop TickerProviderStateMixin and eager controller creation from StyleBuilder; install pointer tracking only for hover/pressed via a needsPointerTracking capability query, and own a controller only to preserve interaction state when an external controller is removed. - Replace the unstable List.sort in mergeActiveVariants with a stable two-bucket partition so variant precedence is deterministic by construction. - Store, query, and notify WidgetState.scrolledUnder in WidgetStateProvider. Adds regression tests for reactive inheritance, aspect-scoped rebuilds, controller ownership, scrolledUnder, and variant-order stability. --- .../core/providers/widget_state_provider.dart | 29 +- packages/mix/lib/src/core/style.dart | 65 +++- packages/mix/lib/src/core/style_builder.dart | 98 +++--- .../src/core/style_get_all_variants_test.dart | 93 ++++++ .../style_resolution_reactivity_test.dart | 280 ++++++++++++++++++ .../widget_state_controller_test.dart | 75 +++++ 6 files changed, 566 insertions(+), 74 deletions(-) create mode 100644 packages/mix/test/src/core/style_resolution_reactivity_test.dart diff --git a/packages/mix/lib/src/core/providers/widget_state_provider.dart b/packages/mix/lib/src/core/providers/widget_state_provider.dart index 7d61f5c65c..81b73ef233 100644 --- a/packages/mix/lib/src/core/providers/widget_state_provider.dart +++ b/packages/mix/lib/src/core/providers/widget_state_provider.dart @@ -11,6 +11,7 @@ extension on Set { bool get hasDragged => contains(WidgetState.dragged); bool get hasSelected => contains(WidgetState.selected); bool get hasError => contains(WidgetState.error); + bool get hasScrolledUnder => contains(WidgetState.scrolledUnder); } /// Provider for widget state information using Flutter's [InheritedModel]. @@ -29,12 +30,17 @@ class WidgetStateProvider extends InheritedModel { pressed = states.hasPressed, dragged = states.hasDragged, selected = states.hasSelected, - error = states.hasError; + error = states.hasError, + scrolledUnder = states.hasScrolledUnder; /// Retrieves the [WidgetStateProvider] from the widget tree. /// /// If [state] is provided, creates a dependency only on that specific state. /// This enables granular rebuilds when only specific states change. + /// + /// Passing a null [state] creates a dependency on *every* aspect, so callers + /// that only need to know whether a scope exists should use [hasProvider] + /// instead of comparing `of(context)` against null. static WidgetStateProvider? of(BuildContext context, [WidgetState? state]) { return InheritedModel.inheritFrom( context, @@ -42,6 +48,15 @@ class WidgetStateProvider extends InheritedModel { ); } + /// Whether a [WidgetStateProvider] scope exists above [context]. + /// + /// Unlike [of], this performs a read-only lookup and does **not** register a + /// dependency on any state aspect, so a widget calling it will not rebuild + /// when unrelated states (hover, press, etc.) change. + static bool hasProvider(BuildContext context) { + return context.getInheritedWidgetOfExactType() != null; + } + /// Checks if a specific [WidgetState] is currently active. /// /// Returns false if no [WidgetStateProvider] is found in the widget tree. @@ -59,7 +74,7 @@ class WidgetStateProvider extends InheritedModel { .dragged => model.dragged, .selected => model.selected, .error => model.error, - .scrolledUnder => false, + .scrolledUnder => model.scrolledUnder, }; } @@ -84,6 +99,9 @@ class WidgetStateProvider extends InheritedModel { /// Whether the widget has an error. final bool error; + /// Whether the widget is scrolled under (e.g. content beneath an app bar). + final bool scrolledUnder; + @override bool updateShouldNotify(WidgetStateProvider oldWidget) { return oldWidget.disabled != disabled || @@ -92,7 +110,8 @@ class WidgetStateProvider extends InheritedModel { oldWidget.pressed != pressed || oldWidget.dragged != dragged || oldWidget.selected != selected || - oldWidget.error != error; + oldWidget.error != error || + oldWidget.scrolledUnder != scrolledUnder; } @override @@ -106,7 +125,9 @@ class WidgetStateProvider extends InheritedModel { oldWidget.pressed != pressed && dependencies.hasPressed || oldWidget.dragged != dragged && dependencies.hasDragged || oldWidget.selected != selected && dependencies.hasSelected || - oldWidget.error != error && dependencies.hasError; + oldWidget.error != error && dependencies.hasError || + oldWidget.scrolledUnder != scrolledUnder && + dependencies.hasScrolledUnder; } } diff --git a/packages/mix/lib/src/core/style.dart b/packages/mix/lib/src/core/style.dart index cb6e268e99..599707e795 100644 --- a/packages/mix/lib/src/core/style.dart +++ b/packages/mix/lib/src/core/style.dart @@ -58,8 +58,14 @@ abstract class Style> extends Mix> } /// Gets the closest [Style] from the widget tree, or null if not found. + /// + /// Registers an inherited-widget dependency, so a widget resolved through + /// this lookup re-resolves when the ancestor [StyleProvider] changes — even + /// if the provider's own child widget instance is identical. This matches the + /// contract of standard Flutter `of` APIs. static Style? maybeOf>(BuildContext context) { - final provider = context.getInheritedWidgetOfExactType>(); + final provider = context + .dependOnInheritedWidgetOfExactType>(); return provider?.style; } @@ -72,6 +78,37 @@ abstract class Style> extends Mix> .toSet(); } + /// Whether this style declares a widget-state variant that automatic pointer + /// tracking can actually produce (`hovered` or `pressed`). + /// + /// [StyleBuilder] uses this to decide whether to install a + /// `MixInteractionDetector`. Other widget states (focus, disabled, selected, + /// dragged, error, scrolledUnder) require an external owner or an existing + /// `WidgetStateProvider` and are intentionally excluded — a pointer detector + /// cannot activate them, so installing one for them would be dead work. + /// + /// This is a boolean capability query rather than a `Set` so the + /// hot path allocates nothing. Only the top-level variants are inspected, + /// matching [widgetStates]; nested variants are not scanned because a pointer + /// detector produces both `hovered` and `pressed` once installed, and any + /// realistic nested pointer variant sits under a top-level pointer variant or + /// under an owner (e.g. `Pressable`) that already provides the scope. + @internal + bool get needsPointerTracking { + final variants = $variants; + if (variants == null) return false; + + for (final variantStyle in variants) { + final variant = variantStyle.variant; + if (variant is WidgetStateVariant && + (variant.state == .hovered || variant.state == .pressed)) { + return true; + } + } + + return false; + } + /// Merges all active variants with their nested variants recursively. /// /// This method evaluates which variants should be active based on the current @@ -88,7 +125,8 @@ abstract class Style> extends Mix> BuildContext context, { required Set namedVariants, }) { - // Filter variants that should be active in this context + // Filter variants that should be active in this context, preserving their + // stored order. final activeVariants = ($variants ?? []) .where( (variantAttr) => switch (variantAttr.variant) { @@ -99,18 +137,25 @@ abstract class Style> extends Mix> ) .toList(); - // Sort by priority: WidgetStateVariant gets applied last (highest priority) - activeVariants.sort( - (a, b) => Comparable.compare( - a.variant is WidgetStateVariant ? 1 : 0, - b.variant is WidgetStateVariant ? 1 : 0, - ), - ); + // Order by priority using a stable linear partition into two buckets, + // preserving stored order within each bucket: + // 1. non-widget-state variants (lower priority, applied first) + // 2. widget-state variants (highest priority, applied last) + // + // A partition is used instead of List.sort because Dart's sort is not + // guaranteed to preserve the relative order of items that compare equal, + // yet Mix's contract requires the last stored variant within a bucket to + // win. Deriving order by construction makes precedence deterministic + // regardless of the sort implementation. + final orderedVariants = [ + ...activeVariants.where((v) => v.variant is! WidgetStateVariant), + ...activeVariants.where((v) => v.variant is WidgetStateVariant), + ]; // Extract the style from each active variant final stylesToMerge = <(Style, bool)>[]; // (style, isFromStyleVariation) - for (final variantAttr in activeVariants) { + for (final variantAttr in orderedVariants) { final result = switch (variantAttr.variant) { ContextVariantBuilder variant => ( variant.build(context) as Style, diff --git a/packages/mix/lib/src/core/style_builder.dart b/packages/mix/lib/src/core/style_builder.dart index ca5fab0eed..bf791a87ef 100644 --- a/packages/mix/lib/src/core/style_builder.dart +++ b/packages/mix/lib/src/core/style_builder.dart @@ -46,45 +46,14 @@ class StyleBuilder> extends StatefulWidget { State> createState() => _StyleBuilderState(); } -class _StyleBuilderState> extends State> - with TickerProviderStateMixin { - late WidgetStatesController _controller; - - /// Tracks whether we created the controller internally (and thus own it) - bool _ownsController = false; - - @override - void initState() { - super.initState(); - _initController(); - } - - void _initController() { - if (widget.controller != null) { - _controller = widget.controller!; - _ownsController = false; - } else { - _controller = WidgetStatesController(); - _ownsController = true; - } - } - - void _handleControllerChange(StyleBuilder oldWidget) { - // Dispose old internal controller if we owned it - if (_ownsController) { - _controller.dispose(); - } - - // Set up new controller - if (widget.controller != null) { - _controller = widget.controller!; - _ownsController = false; - } else { - // Create internal controller, preserving state from old external controller - _controller = WidgetStatesController(oldWidget.controller?.value ?? {}); - _ownsController = true; - } - } +class _StyleBuilderState> extends State> { + /// Controller owned by this state, created lazily and only to preserve + /// interaction state when an external [StyleBuilder.controller] is removed. + /// + /// It stays null on the common paths: a state-free style allocates nothing, + /// and when automatic tracking is installed without a prior external + /// controller the [MixInteractionDetector] owns its own controller instead. + WidgetStatesController? _preservedController; Style _buildStyle(BuildContext context) { final inheritedStyle = Style.maybeOf(context); @@ -105,31 +74,33 @@ class _StyleBuilderState> extends State> void didUpdateWidget(covariant StyleBuilder oldWidget) { super.didUpdateWidget(oldWidget); - // Handle controller changes - if (oldWidget.controller != widget.controller) { - _handleControllerChange(oldWidget); + if (oldWidget.controller == widget.controller) return; + + if (widget.controller != null) { + // An external controller now drives state; drop any internal one we own. + _preservedController?.dispose(); + _preservedController = null; + } else if (widget.style.needsPointerTracking) { + // The external controller was removed while the style still reacts to + // pointer state. Seed an internal controller from its last states so a + // transient hover/press does not reset when swapping to automatic + // tracking. + _preservedController = WidgetStatesController( + oldWidget.controller?.value ?? {}, + ); } } @override void dispose() { - // Only dispose controllers we created internally - if (_ownsController) { - _controller.dispose(); - } - + _preservedController?.dispose(); super.dispose(); } @override Widget build(BuildContext context) { final style = _buildStyle(context); - - // Calculate interactivity need early - final needsToTrackWidgetState = - widget.controller == null && style.widgetStates.isNotEmpty; - - final alreadyHasWidgetStateScope = WidgetStateProvider.of(context) != null; + final externalController = widget.controller; Widget current = Builder( builder: (context) { @@ -142,14 +113,21 @@ class _StyleBuilderState> extends State> }, ); - if (needsToTrackWidgetState && !alreadyHasWidgetStateScope) { - // If we need interactivity and no MixWidgetStateModel is present, - // wrap in MixInteractionDetector - current = MixInteractionDetector(controller: _controller, child: current); - } else if (widget.controller != null) { - // If we have an external controller, wrap with _ExternalControllerProvider + if (externalController != null) { + // An external controller drives state; mirror it into the subtree. current = _ExternalControllerProvider( - controller: _controller, + controller: externalController, + child: current, + ); + } else if (style.needsPointerTracking && + !WidgetStateProvider.hasProvider(context)) { + // Install automatic pointer tracking only when the style reacts to a + // pointer-produced state (hover/press) and no ancestor already provides a + // WidgetStateProvider scope. Reuse a preserved controller if we own one + // (external controller was just removed); otherwise let the detector + // create and own its controller so nothing is allocated here. + current = MixInteractionDetector( + controller: _preservedController, child: current, ); } diff --git a/packages/mix/test/src/core/style_get_all_variants_test.dart b/packages/mix/test/src/core/style_get_all_variants_test.dart index 29a892ee12..4fe325165b 100644 --- a/packages/mix/test/src/core/style_get_all_variants_test.dart +++ b/packages/mix/test/src/core/style_get_all_variants_test.dart @@ -640,6 +640,99 @@ void main() { }); }); + group('Stable ordering (linear partition)', () { + testWidgets( + 'interleaved buckets preserve stored order within each bucket', + (tester) async { + // Interleave non-widget-state and widget-state variants. After the + // partition, the last stored variant within each bucket must win the + // merge: height from the last non-state variant, width from the last + // widget-state variant. + final testAttribute = _MockSpecAttribute( + width: 50.0, + height: 75.0, + variants: [ + VariantStyle( + ContextVariant('ctx1', (_) => true), + _MockSpecAttribute(width: 0.0, height: 10.0), + ), + VariantStyle( + WidgetStateVariant(WidgetState.hovered), + _MockSpecAttribute(width: 100.0), + ), + VariantStyle( + ContextVariant('ctx2', (_) => true), + _MockSpecAttribute(width: 0.0, height: 20.0), + ), + VariantStyle( + WidgetStateVariant(WidgetState.pressed), + _MockSpecAttribute(width: 200.0), + ), + VariantStyle( + ContextVariant('ctx3', (_) => true), + _MockSpecAttribute(width: 0.0, height: 30.0), + ), + ], + ); + + await tester.pumpWidget( + MaterialApp( + home: WidgetStateProvider( + states: const {WidgetState.hovered, WidgetState.pressed}, + child: Builder( + builder: (context) { + final result = testAttribute.mergeActiveVariants( + context, + namedVariants: {}, + ); + final spec = result.resolve(context); + final resolved = spec.resolvedValue as Map; + + // Last non-state variant wins height. + expect(resolved['height'], 30.0); + // Last widget-state variant wins width. + expect(resolved['width'], 200.0); + + return Container(); + }, + ), + ), + ), + ); + }, + ); + + test('large equal-priority list preserves stored order', () { + // 50 equally-prioritized (all active) context variants. A stable + // partition keeps stored order, so the last-stored variant wins the + // merge. An unstable List.sort would not guarantee this for a list this + // large (Dart falls back to a non-stable sort above ~32 elements). + const count = 50; + final variants = [ + for (var i = 1; i <= count; i++) + VariantStyle>>( + ContextVariant('ctx$i', (_) => true), + _MockSpecAttribute(width: 0.0, height: i.toDouble()), + ), + ]; + + final testAttribute = _MockSpecAttribute( + width: 50.0, + variants: variants, + ); + + final context = MockBuildContext(); + final result = testAttribute.mergeActiveVariants( + context, + namedVariants: {}, + ); + final spec = result.resolve(context); + + // The last stored variant (height == count) must win. + expect((spec.resolvedValue as Map)['height'], count.toDouble()); + }); + }); + group('Integration with existing variant system', () { testWidgets('works with actual WidgetStateVariant instances', ( tester, diff --git a/packages/mix/test/src/core/style_resolution_reactivity_test.dart b/packages/mix/test/src/core/style_resolution_reactivity_test.dart new file mode 100644 index 0000000000..0638749717 --- /dev/null +++ b/packages/mix/test/src/core/style_resolution_reactivity_test.dart @@ -0,0 +1,280 @@ +import 'package:flutter/material.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:mix/mix.dart'; +import 'package:mix/src/core/internal/mix_interaction_detector.dart'; + +/// Regression tests for reactive inherited-style lookup and widget-state +/// dependency ownership. +/// +/// These pin the corrections described in the "style resolution correctness" +/// change: inherited styles must react to provider changes, and a +/// state-independent style must not depend on widget-state changes above it. +void main() { + Color? colorOf(BoxSpec spec) => (spec.decoration as BoxDecoration?)?.color; + + group('Inherited style reactivity', () { + testWidgets( + 'a StyleProvider change re-resolves an identical child StyleBuilder', + (tester) async { + late BoxSpec resolved; + + // Captured once, so the child StyleBuilder instance is identical across + // parent rebuilds. Only an inherited-widget dependency can make it + // re-resolve — a stable child cannot rely on its own rebuild. + final child = StyleBuilder( + style: BoxStyler().width(10), + builder: (context, spec) { + resolved = spec; + return const SizedBox(); + }, + ); + + late StateSetter setOuter; + var providerColor = Colors.red; + + await tester.pumpWidget( + MaterialApp( + home: StatefulBuilder( + builder: (context, setState) { + setOuter = setState; + + return StyleProvider( + style: BoxStyler().color(providerColor), + child: child, + ); + }, + ), + ), + ); + + expect(colorOf(resolved), Colors.red); + + setOuter(() => providerColor = Colors.green); + await tester.pump(); + + expect( + colorOf(resolved), + Colors.green, + reason: 'inherited style change must re-resolve even a stable child', + ); + }, + ); + }); + + group('Widget-state dependency ownership', () { + testWidgets( + 'a state-independent style does not re-resolve on ancestor state changes', + (tester) async { + final controller = WidgetStatesController(); + addTearDown(controller.dispose); + + var buildCount = 0; + + // Identical child across provider rebuilds isolates the dependency: + // it re-resolves only if it subscribed to the WidgetStateProvider. + final child = StyleBuilder( + style: BoxStyler().color(Colors.blue), + builder: (context, spec) { + buildCount++; + return const SizedBox(); + }, + ); + + await tester.pumpWidget( + MaterialApp( + home: ListenableBuilder( + listenable: controller, + builder: (context, _) { + return WidgetStateProvider( + states: controller.value, + child: child, + ); + }, + ), + ), + ); + + final baseline = buildCount; + + controller.hovered = true; + await tester.pump(); + expect( + buildCount, + baseline, + reason: 'hover change must not re-resolve a state-free style', + ); + + controller.pressed = true; + await tester.pump(); + expect( + buildCount, + baseline, + reason: 'press change must not re-resolve a state-free style', + ); + + controller.focused = true; + await tester.pump(); + expect( + buildCount, + baseline, + reason: 'focus change must not re-resolve a state-free style', + ); + }, + ); + + testWidgets('a pressed-only style depends on pressed but not hover', ( + tester, + ) async { + final controller = WidgetStatesController(); + addTearDown(controller.dispose); + + var buildCount = 0; + Color? color; + + final child = StyleBuilder( + style: BoxStyler() + .color(Colors.blue) + .onPressed(BoxStyler().color(Colors.red)), + builder: (context, spec) { + buildCount++; + color = colorOf(spec); + return const SizedBox(); + }, + ); + + await tester.pumpWidget( + MaterialApp( + home: ListenableBuilder( + listenable: controller, + builder: (context, _) { + return WidgetStateProvider( + states: controller.value, + child: child, + ); + }, + ), + ), + ); + + final baseline = buildCount; + expect(color, Colors.blue); + + // Hover is unrelated to a pressed-only style: no re-resolution. + controller.hovered = true; + await tester.pump(); + expect( + buildCount, + baseline, + reason: 'pressed-only style must not depend on hover', + ); + expect(color, Colors.blue); + + // Pressed is a declared dependency: must re-resolve. + controller.pressed = true; + await tester.pump(); + expect( + buildCount, + greaterThan(baseline), + reason: 'pressed-only style must react to pressed state', + ); + expect(color, Colors.red); + }); + }); + + group('Interaction machinery installation', () { + testWidgets('a state-free style installs no controller or detector', ( + tester, + ) async { + await tester.pumpWidget( + MaterialApp( + home: StyleBuilder( + style: BoxStyler().color(Colors.blue).width(10).height(10), + builder: (context, spec) => const SizedBox(), + ), + ), + ); + + // No detector and no WidgetStateProvider means no controller was created + // for a style that never reacts to widget state. + expect(find.byType(MixInteractionDetector), findsNothing); + expect(find.byType(WidgetStateProvider), findsNothing); + }); + + testWidgets( + 'a focus-only style installs no pointer detector that cannot activate it', + (tester) async { + await tester.pumpWidget( + MaterialApp( + home: StyleBuilder( + style: BoxStyler() + .color(Colors.blue) + .onFocused(BoxStyler().color(Colors.red)), + builder: (context, spec) => const SizedBox(), + ), + ), + ); + + // A pointer detector produces hover/press, never focus, so installing + // one for a focus-only style would be dead work. + expect(find.byType(MixInteractionDetector), findsNothing); + }, + ); + + testWidgets('a hover style installs exactly one pointer detector', ( + tester, + ) async { + await tester.pumpWidget( + MaterialApp( + home: StyleBuilder( + style: BoxStyler() + .color(Colors.blue) + .onHovered(BoxStyler().color(Colors.red)), + builder: (context, spec) => const SizedBox(), + ), + ), + ); + + expect(find.byType(MixInteractionDetector), findsOneWidget); + }); + }); + + group('scrolledUnder through the public state path', () { + testWidgets('activates through an external WidgetStatesController', ( + tester, + ) async { + final controller = WidgetStatesController(); + addTearDown(controller.dispose); + + Color? color; + + await tester.pumpWidget( + MaterialApp( + home: StyleBuilder( + controller: controller, + style: BoxStyler() + .color(Colors.blue) + .variant( + ContextVariant.widgetState(WidgetState.scrolledUnder), + BoxStyler().color(Colors.red), + ), + builder: (context, spec) { + color = colorOf(spec); + return const SizedBox(); + }, + ), + ), + ); + + expect(color, Colors.blue); + + controller.scrolledUnder = true; + await tester.pump(); + + expect( + color, + Colors.red, + reason: 'scrolledUnder must resolve through the state variant path', + ); + }); + }); +} diff --git a/packages/mix/test/src/specs/pressable/widget_state_controller_test.dart b/packages/mix/test/src/specs/pressable/widget_state_controller_test.dart index c7eae73299..bf222674d3 100644 --- a/packages/mix/test/src/specs/pressable/widget_state_controller_test.dart +++ b/packages/mix/test/src/specs/pressable/widget_state_controller_test.dart @@ -217,6 +217,81 @@ void main() { isFalse, ); }); + + testWidgets('hasStateOf reports scrolledUnder', (tester) async { + await tester.pumpWidget( + MaterialApp( + home: WidgetStateProvider( + states: {WidgetState.scrolledUnder}, + child: Builder( + builder: (context) { + expect( + WidgetStateProvider.hasStateOf( + context, + WidgetState.scrolledUnder, + ), + isTrue, + ); + expect( + WidgetStateProvider.hasStateOf(context, WidgetState.hovered), + isFalse, + ); + return Container(); + }, + ), + ), + ), + ); + }); + + test('scrolledUnder participates in dependent notifications', () { + final oldModel = WidgetStateProvider(states: {}, child: Container()); + final newModel = WidgetStateProvider( + states: {WidgetState.scrolledUnder}, + child: Container(), + ); + + expect(newModel.updateShouldNotify(oldModel), isTrue); + expect( + newModel.updateShouldNotifyDependent(oldModel, { + WidgetState.scrolledUnder, + }), + isTrue, + ); + expect( + newModel.updateShouldNotifyDependent(oldModel, {WidgetState.hovered}), + isFalse, + ); + }); + + testWidgets('hasProvider detects an ancestor scope', (tester) async { + await tester.pumpWidget( + MaterialApp( + home: WidgetStateProvider( + states: {WidgetState.hovered}, + child: Builder( + builder: (context) { + expect(WidgetStateProvider.hasProvider(context), isTrue); + return Container(); + }, + ), + ), + ), + ); + }); + + testWidgets('hasProvider returns false without a scope', (tester) async { + await tester.pumpWidget( + MaterialApp( + home: Builder( + builder: (context) { + expect(WidgetStateProvider.hasProvider(context), isFalse); + return Container(); + }, + ), + ), + ); + }); }); testWidgets('PressableState updates widgets correctly', ( From d85cd59ffb80b23e9d35415f9b92e9134d5c9d14 Mon Sep 17 00:00:00 2001 From: Leo Farias Date: Sat, 11 Jul 2026 12:42:04 -0400 Subject: [PATCH 2/4] fix(mix): harden automatic pointer state tracking --- packages/mix/lib/src/core/style.dart | 54 +++++++++-------- packages/mix/lib/src/core/style_builder.dart | 41 +++++++++---- .../src/core/style_builder_hover_test.dart | 7 --- .../mix/test/src/core/style_builder_test.dart | 46 +++++++++++++++ .../src/core/style_get_all_variants_test.dart | 24 ++++---- .../style_resolution_reactivity_test.dart | 59 +++++++++++++++++++ 6 files changed, 177 insertions(+), 54 deletions(-) diff --git a/packages/mix/lib/src/core/style.dart b/packages/mix/lib/src/core/style.dart index 599707e795..1380d0fa34 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'; @@ -70,12 +72,26 @@ abstract class Style> extends Mix> return provider?.style; } - @internal - Set get widgetStates { - return ($variants ?? []) - .where((v) => v.variant is WidgetStateVariant) - .map((v) => (v.variant as WidgetStateVariant).state) - .toSet(); + bool _needsPointerTracking(Set> visited) { + if (!visited.add(this)) return false; + + final variants = $variants; + if (variants == null) return false; + + for (final variantStyle in variants) { + final variant = variantStyle.variant; + if (variant is WidgetStateVariant && + (variant.state == .hovered || variant.state == .pressed)) { + return true; + } + + if (variant is! ContextVariantBuilder && + variantStyle.value._needsPointerTracking(visited)) { + return true; + } + } + + return false; } /// Whether this style declares a widget-state variant that automatic pointer @@ -87,26 +103,15 @@ abstract class Style> extends Mix> /// `WidgetStateProvider` and are intentionally excluded — a pointer detector /// cannot activate them, so installing one for them would be dead work. /// - /// This is a boolean capability query rather than a `Set` so the - /// hot path allocates nothing. Only the top-level variants are inspected, - /// matching [widgetStates]; nested variants are not scanned because a pointer - /// detector produces both `hovered` and `pressed` once installed, and any - /// realistic nested pointer variant sits under a top-level pointer variant or - /// under an owner (e.g. `Pressable`) that already provides the scope. + /// Static nested variant branches are inspected recursively. Dynamic + /// [ContextVariantBuilder] output remains opaque and requires an external + /// state owner when it returns pointer-state variants. @internal bool get needsPointerTracking { final variants = $variants; - if (variants == null) return false; + if (variants == null || variants.isEmpty) return false; - for (final variantStyle in variants) { - final variant = variantStyle.variant; - if (variant is WidgetStateVariant && - (variant.state == .hovered || variant.state == .pressed)) { - return true; - } - } - - return false; + return _needsPointerTracking(HashSet.identity()); } /// Merges all active variants with their nested variants recursively. @@ -125,9 +130,12 @@ abstract class Style> extends Mix> BuildContext context, { required Set namedVariants, }) { + final variants = $variants; + if (variants == null || variants.isEmpty) return this; + // Filter variants that should be active in this context, preserving their // stored order. - final activeVariants = ($variants ?? []) + final activeVariants = variants .where( (variantAttr) => switch (variantAttr.variant) { (ContextVariant variant) => variant.when(context), diff --git a/packages/mix/lib/src/core/style_builder.dart b/packages/mix/lib/src/core/style_builder.dart index bf791a87ef..1d1d38b7e5 100644 --- a/packages/mix/lib/src/core/style_builder.dart +++ b/packages/mix/lib/src/core/style_builder.dart @@ -55,6 +55,27 @@ class _StyleBuilderState> extends State> { /// controller the [MixInteractionDetector] owns its own controller instead. WidgetStatesController? _preservedController; + /// State captured when an external controller is removed. The snapshot is + /// consumed only if the next build installs automatic pointer tracking. + Set? _controllerHandoffStates; + + WidgetStatesController? _controllerForAutomaticTracking() { + final states = _controllerHandoffStates; + if (states == null) return _preservedController; + + _controllerHandoffStates = null; + _preservedController?.dispose(); + _preservedController = WidgetStatesController(states); + + return _preservedController; + } + + void _clearControllerHandoff() { + _controllerHandoffStates = null; + _preservedController?.dispose(); + _preservedController = null; + } + Style _buildStyle(BuildContext context) { final inheritedStyle = Style.maybeOf(context); final style = widget.style; @@ -77,23 +98,15 @@ class _StyleBuilderState> extends State> { if (oldWidget.controller == widget.controller) return; if (widget.controller != null) { - // An external controller now drives state; drop any internal one we own. - _preservedController?.dispose(); - _preservedController = null; - } else if (widget.style.needsPointerTracking) { - // The external controller was removed while the style still reacts to - // pointer state. Seed an internal controller from its last states so a - // transient hover/press does not reset when swapping to automatic - // tracking. - _preservedController = WidgetStatesController( - oldWidget.controller?.value ?? {}, - ); + _clearControllerHandoff(); + } else { + _controllerHandoffStates = Set.of(oldWidget.controller!.value); } } @override void dispose() { - _preservedController?.dispose(); + _clearControllerHandoff(); super.dispose(); } @@ -127,9 +140,11 @@ class _StyleBuilderState> extends State> { // (external controller was just removed); otherwise let the detector // create and own its controller so nothing is allocated here. current = MixInteractionDetector( - controller: _preservedController, + controller: _controllerForAutomaticTracking(), child: current, ); + } else { + _clearControllerHandoff(); } // If inheritable is true, wrap with StyleProvider to pass the merged style down 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..4909b0db6d 100644 --- a/packages/mix/test/src/core/style_builder_hover_test.dart +++ b/packages/mix/test/src/core/style_builder_hover_test.dart @@ -23,13 +23,6 @@ void main() { Color? currentColor; - // Verify the style has widget states - expect( - style.widgetStates.isNotEmpty, - isTrue, - reason: 'Style should have widget states for hover', - ); - await tester.pumpWidget( MaterialApp( home: Scaffold( diff --git a/packages/mix/test/src/core/style_builder_test.dart b/packages/mix/test/src/core/style_builder_test.dart index 1d4421e05a..3b2fba9c87 100644 --- a/packages/mix/test/src/core/style_builder_test.dart +++ b/packages/mix/test/src/core/style_builder_test.dart @@ -839,6 +839,52 @@ void main() { ); }, ); + + testWidgets( + 'preserves inherited widget state when external controller is removed', + (tester) async { + final externalController = WidgetStatesController({ + WidgetState.hovered, + }); + addTearDown(externalController.dispose); + + WidgetStatesController? controller = externalController; + late StateSetter setOuter; + Color? color; + + await tester.pumpWidget( + MaterialApp( + home: StyleProvider( + style: BoxStyler() + .color(Colors.red) + .onHovered(BoxStyler().color(Colors.blue)), + child: StatefulBuilder( + builder: (context, setState) { + setOuter = setState; + + return StyleBuilder( + controller: controller, + style: BoxStyler().size(100, 100), + builder: (context, spec) { + color = (spec.decoration as BoxDecoration?)?.color; + + return const SizedBox(width: 100, height: 100); + }, + ); + }, + ), + ), + ), + ); + + expect(color, Colors.blue); + + setOuter(() => controller = null); + await tester.pump(); + + expect(color, Colors.blue); + }, + ); }); testWidgets( diff --git a/packages/mix/test/src/core/style_get_all_variants_test.dart b/packages/mix/test/src/core/style_get_all_variants_test.dart index 4fe325165b..949c985dc1 100644 --- a/packages/mix/test/src/core/style_get_all_variants_test.dart +++ b/packages/mix/test/src/core/style_get_all_variants_test.dart @@ -35,7 +35,7 @@ void main() { group('Style.mergeActiveVariants', () { group('Variant priority system', () { - testWidgets('WidgetStateVariant gets sorted last (highest priority)', ( + testWidgets('WidgetStateVariant is applied last (highest priority)', ( tester, ) async { // Create test attribute with mixed variant types @@ -63,7 +63,7 @@ void main() { width: 50.0, variants: [ contextVarAttr, - widgetStateVarAttr, // Should be sorted last + widgetStateVarAttr, // Should be applied last namedVarAttr, ], ); @@ -146,7 +146,9 @@ void main() { ); }); - testWidgets('mixed variant types are sorted correctly', (tester) async { + testWidgets('mixed variant types follow priority buckets', ( + tester, + ) async { // Create a mix of all variant types final contextVariant = ContextVariant('context', (context) => true); const namedVariant = NamedVariant('named'); @@ -158,23 +160,23 @@ void main() { VariantStyle( widgetStateVariant1, _MockSpecAttribute(width: 100.0), - ), // Should be sorted to end + ), // Higher-priority bucket VariantStyle( contextVariant, _MockSpecAttribute(width: 200.0), - ), // Should be sorted earlier + ), // Lower-priority bucket VariantStyle( widgetStateVariant2, _MockSpecAttribute(width: 300.0), - ), // Should be sorted to end + ), // Higher-priority bucket VariantStyle( namedVariant, _MockSpecAttribute(width: 400.0), - ), // Should be sorted earlier + ), // Lower-priority bucket VariantStyle( multiNamedVariant, _MockSpecAttribute(width: 500.0), - ), // Should be sorted earlier + ), // Lower-priority bucket ]; final testAttribute = _MockSpecAttribute( @@ -352,7 +354,7 @@ void main() { }); group('Merging behavior', () { - testWidgets('variants are merged in sorted order', (tester) async { + testWidgets('variants are merged in priority order', (tester) async { final contextVariant = ContextVariant('context', (context) => true); final widgetStateVariant = WidgetStateVariant(WidgetState.hovered); @@ -599,8 +601,8 @@ void main() { expect((spec.resolvedValue as Map)['width'], 50.0); }); - test('sorting is stable for non-WidgetStateVariant elements', () { - // Create multiple non-WidgetState variants to test stable sort + test('non-WidgetStateVariant elements preserve stored order', () { + // Multiple non-widget-state variants remain in their stored order. final variants = [ VariantStyle( ContextVariant('context1', (context) => true), diff --git a/packages/mix/test/src/core/style_resolution_reactivity_test.dart b/packages/mix/test/src/core/style_resolution_reactivity_test.dart index 0638749717..695910c2cd 100644 --- a/packages/mix/test/src/core/style_resolution_reactivity_test.dart +++ b/packages/mix/test/src/core/style_resolution_reactivity_test.dart @@ -1,3 +1,4 @@ +import 'package:flutter/gestures.dart'; import 'package:flutter/material.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:mix/mix.dart'; @@ -236,6 +237,64 @@ void main() { expect(find.byType(MixInteractionDetector), findsOneWidget); }); + + testWidgets('a statically nested hover variant tracks pointer state', ( + tester, + ) async { + Color? color; + + await tester.pumpWidget( + MaterialApp( + home: Center( + child: StyleBuilder( + style: BoxStyler() + .color(Colors.blue) + .onLight( + BoxStyler().onHovered(BoxStyler().color(Colors.red)), + ), + builder: (context, spec) { + color = colorOf(spec); + + return const SizedBox( + key: Key('nested-hover-target'), + width: 100, + height: 100, + ); + }, + ), + ), + ), + ); + + expect(color, Colors.blue); + expect(find.byType(MixInteractionDetector), findsOneWidget); + + final pointer = await tester.createGesture(kind: PointerDeviceKind.mouse); + await pointer.addPointer(location: Offset.zero); + await pointer.moveTo( + tester.getCenter(find.byKey(const Key('nested-hover-target'))), + ); + await tester.pump(); + + expect(color, Colors.red); + await pointer.removePointer(); + }); + + test('static pointer capability handles cyclic variant graphs', () { + final variants = >[]; + final style = BoxStyler(variants: variants); + + variants + ..add(VariantStyle(ContextVariant('cycle', (_) => true), style)) + ..add( + VariantStyle( + ContextVariant('nested-hover', (_) => true), + BoxStyler().onHovered(BoxStyler().color(Colors.red)), + ), + ); + + expect(style.needsPointerTracking, isTrue); + }); }); group('scrolledUnder through the public state path', () { From 62e881f90615fc1ed451f118671ce72dcd1b89c2 Mon Sep 17 00:00:00 2001 From: Leo Farias Date: Sat, 11 Jul 2026 13:30:20 -0400 Subject: [PATCH 3/4] refactor(mix): centralize interaction controller ownership --- .../internal/mix_interaction_detector.dart | 23 ++-- packages/mix/lib/src/core/style_builder.dart | 40 ++----- .../mix_interaction_detector_test.dart | 103 ++++++++++++++++++ .../mix/test/src/core/style_builder_test.dart | 52 +++++++++ 4 files changed, 176 insertions(+), 42 deletions(-) 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..51e3524c13 100644 --- a/packages/mix/lib/src/core/internal/mix_interaction_detector.dart +++ b/packages/mix/lib/src/core/internal/mix_interaction_detector.dart @@ -18,6 +18,7 @@ class MixInteractionDetector extends StatefulWidget { super.key, required this.child, this.controller, + this.initialStates = const {}, this.enabled = true, this.onHoverChange, this.onPointerPositionChange, @@ -25,6 +26,13 @@ class MixInteractionDetector extends StatefulWidget { final Widget child; final WidgetStatesController? controller; + + /// States used to initialize the internally owned controller. + /// + /// These are ignored when [controller] is provided and are not reapplied on + /// subsequent widget updates. + final Set initialStates; + final bool enabled; final ValueChanged? onHoverChange; final ValueChanged? onPointerPositionChange; @@ -41,14 +49,12 @@ class _MixInteractionDetectorState extends State { void initState() { super.initState(); _cursorPositionNotifier = PointerPositionNotifier(); + if (widget.controller == null) { + _internalController = WidgetStatesController(widget.initialStates); + } _syncDisabledState(); } - /// Creates an internal controller with initial disabled state if needed. - WidgetStatesController _createInternalController() { - return WidgetStatesController({if (!widget.enabled) .disabled}); - } - /// Syncs disabled state and clears transients when disabling. void _syncDisabledState() { _effectiveController.update(.disabled, !widget.enabled); @@ -63,9 +69,7 @@ class _MixInteractionDetectorState extends State { /// Handles state controller changes between external and internal. void _handleControllerChange(MixInteractionDetector oldWidget) { if (widget.controller == null) { - _internalController ??= WidgetStatesController( - oldWidget.controller?.value ?? {}, - ); + _internalController = WidgetStatesController(oldWidget.controller!.value); } else { _internalController?.dispose(); _internalController = null; @@ -165,8 +169,7 @@ class _MixInteractionDetectorState extends State { } WidgetStatesController get _effectiveController => - widget.controller ?? - (_internalController ??= _createInternalController()); + widget.controller ?? _internalController!; @override void didUpdateWidget(MixInteractionDetector oldWidget) { diff --git a/packages/mix/lib/src/core/style_builder.dart b/packages/mix/lib/src/core/style_builder.dart index 1d1d38b7e5..9efff57660 100644 --- a/packages/mix/lib/src/core/style_builder.dart +++ b/packages/mix/lib/src/core/style_builder.dart @@ -47,33 +47,15 @@ class StyleBuilder> extends StatefulWidget { } class _StyleBuilderState> extends State> { - /// Controller owned by this state, created lazily and only to preserve - /// interaction state when an external [StyleBuilder.controller] is removed. - /// - /// It stays null on the common paths: a state-free style allocates nothing, - /// and when automatic tracking is installed without a prior external - /// controller the [MixInteractionDetector] owns its own controller instead. - WidgetStatesController? _preservedController; - /// State captured when an external controller is removed. The snapshot is /// consumed only if the next build installs automatic pointer tracking. Set? _controllerHandoffStates; - WidgetStatesController? _controllerForAutomaticTracking() { + Set _takeControllerHandoffStates() { final states = _controllerHandoffStates; - if (states == null) return _preservedController; - _controllerHandoffStates = null; - _preservedController?.dispose(); - _preservedController = WidgetStatesController(states); - return _preservedController; - } - - void _clearControllerHandoff() { - _controllerHandoffStates = null; - _preservedController?.dispose(); - _preservedController = null; + return states ?? const {}; } Style _buildStyle(BuildContext context) { @@ -98,18 +80,12 @@ class _StyleBuilderState> extends State> { if (oldWidget.controller == widget.controller) return; if (widget.controller != null) { - _clearControllerHandoff(); + _controllerHandoffStates = null; } else { _controllerHandoffStates = Set.of(oldWidget.controller!.value); } } - @override - void dispose() { - _clearControllerHandoff(); - super.dispose(); - } - @override Widget build(BuildContext context) { final style = _buildStyle(context); @@ -136,15 +112,15 @@ class _StyleBuilderState> extends State> { !WidgetStateProvider.hasProvider(context)) { // Install automatic pointer tracking only when the style reacts to a // pointer-produced state (hover/press) and no ancestor already provides a - // WidgetStateProvider scope. Reuse a preserved controller if we own one - // (external controller was just removed); otherwise let the detector - // create and own its controller so nothing is allocated here. + // WidgetStateProvider scope. A state snapshot is passed only when an + // external controller was just removed; the detector creates and owns the + // automatic controller in every case. current = MixInteractionDetector( - controller: _controllerForAutomaticTracking(), + initialStates: _takeControllerHandoffStates(), child: current, ); } else { - _clearControllerHandoff(); + _controllerHandoffStates = null; } // If inheritable is true, wrap with StyleProvider to pass the merged style down diff --git a/packages/mix/test/src/core/internal/mix_interaction_detector_test.dart b/packages/mix/test/src/core/internal/mix_interaction_detector_test.dart index 6a5b7da6b5..6e87dc208c 100644 --- a/packages/mix/test/src/core/internal/mix_interaction_detector_test.dart +++ b/packages/mix/test/src/core/internal/mix_interaction_detector_test.dart @@ -5,8 +5,111 @@ import 'package:mix/src/core/internal/mix_interaction_detector.dart'; import 'package:mix/src/core/pointer_position.dart'; import 'package:mix/src/core/providers/widget_state_provider.dart'; +Widget _stateProbe(ValueChanged onBuild) { + return Builder( + builder: (context) { + onBuild(WidgetStateProvider.of(context)!); + + return const SizedBox(width: 100, height: 100); + }, + ); +} + void main() { group('MixInteractionDetector', () { + testWidgets('seeds an internally owned controller from initial states', ( + tester, + ) async { + WidgetStateProvider? states; + + await tester.pumpWidget( + MaterialApp( + home: MixInteractionDetector( + initialStates: const {WidgetState.hovered, WidgetState.focused}, + child: _stateProbe((value) => states = value), + ), + ), + ); + + expect(states?.hovered, isTrue); + expect(states?.focused, isTrue); + }); + + testWidgets('does not reapply initial states after initialization', ( + tester, + ) async { + WidgetStateProvider? states; + + Widget buildDetector(Set initialStates) { + return MaterialApp( + home: MixInteractionDetector( + initialStates: initialStates, + child: _stateProbe((value) => states = value), + ), + ); + } + + await tester.pumpWidget(buildDetector(const {WidgetState.hovered})); + expect(states?.hovered, isTrue); + expect(states?.pressed, isFalse); + + await tester.pumpWidget(buildDetector(const {WidgetState.pressed})); + expect(states?.hovered, isTrue); + expect(states?.pressed, isFalse); + }); + + testWidgets('external controller takes precedence over initial states', ( + tester, + ) async { + final externalController = WidgetStatesController({WidgetState.selected}); + addTearDown(externalController.dispose); + + WidgetStateProvider? states; + + Widget buildDetector(WidgetStatesController? controller) { + return MaterialApp( + home: MixInteractionDetector( + controller: controller, + initialStates: const {WidgetState.hovered}, + child: _stateProbe((value) => states = value), + ), + ); + } + + await tester.pumpWidget(buildDetector(externalController)); + expect(states?.hovered, isFalse); + expect(states?.selected, isTrue); + + await tester.pumpWidget(buildDetector(null)); + expect(states?.hovered, isFalse); + expect(states?.selected, isTrue); + }); + + testWidgets('disabled state normalizes transient initial states', ( + tester, + ) async { + WidgetStateProvider? states; + + await tester.pumpWidget( + MaterialApp( + home: MixInteractionDetector( + enabled: false, + initialStates: const { + WidgetState.hovered, + WidgetState.pressed, + WidgetState.focused, + }, + child: _stateProbe((value) => states = value), + ), + ), + ); + + expect(states?.disabled, isTrue); + expect(states?.hovered, isFalse); + expect(states?.pressed, isFalse); + expect(states?.focused, isTrue); + }); + testWidgets('initializes with disabled state when enabled is false', ( WidgetTester tester, ) async { diff --git a/packages/mix/test/src/core/style_builder_test.dart b/packages/mix/test/src/core/style_builder_test.dart index 3b2fba9c87..dd2316c765 100644 --- a/packages/mix/test/src/core/style_builder_test.dart +++ b/packages/mix/test/src/core/style_builder_test.dart @@ -885,6 +885,58 @@ void main() { expect(color, Colors.blue); }, ); + + testWidgets( + 'discards handoff state when automatic tracking is not installed', + (tester) async { + final externalController = WidgetStatesController({ + WidgetState.hovered, + }); + addTearDown(externalController.dispose); + + WidgetStatesController? controller = externalController; + var tracksHover = false; + late StateSetter setOuter; + Color? color; + + await tester.pumpWidget( + MaterialApp( + home: StatefulBuilder( + builder: (context, setState) { + setOuter = setState; + + final style = BoxStyler().color(Colors.red); + + return StyleBuilder( + controller: controller, + style: tracksHover + ? style.onHovered(BoxStyler().color(Colors.blue)) + : style, + builder: (context, spec) { + color = (spec.decoration as BoxDecoration?)?.color; + + return const SizedBox(width: 100, height: 100); + }, + ); + }, + ), + ), + ); + + expect(color, Colors.red); + + setOuter(() => controller = null); + await tester.pump(); + + expect(find.byType(MixInteractionDetector), findsNothing); + + setOuter(() => tracksHover = true); + await tester.pump(); + + expect(find.byType(MixInteractionDetector), findsOneWidget); + expect(color, Colors.red); + }, + ); }); testWidgets( From 74c78819dbeb0a547dd507372c935b44cbe34921 Mon Sep 17 00:00:00 2001 From: Leo Farias Date: Sat, 11 Jul 2026 13:56:16 -0400 Subject: [PATCH 4/4] docs(mix): clarify interaction controller lifecycle --- .../lib/src/core/internal/mix_interaction_detector.dart | 8 +++++--- packages/mix/lib/src/core/style_builder.dart | 7 +++++-- 2 files changed, 10 insertions(+), 5 deletions(-) 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 51e3524c13..4073a1f502 100644 --- a/packages/mix/lib/src/core/internal/mix_interaction_detector.dart +++ b/packages/mix/lib/src/core/internal/mix_interaction_detector.dart @@ -27,10 +27,12 @@ class MixInteractionDetector extends StatefulWidget { final Widget child; final WidgetStatesController? controller; - /// States used to initialize the internally owned controller. + /// States used to seed an internally owned controller. /// - /// These are ignored when [controller] is provided and are not reapplied on - /// subsequent widget updates. + /// This set is read only when the detector is first mounted without an + /// external [controller]. The states are copied before [enabled] is + /// synchronized, which clears hover and press while disabled. Later changes + /// to this property are not reapplied to the active controller. final Set initialStates; final bool enabled; diff --git a/packages/mix/lib/src/core/style_builder.dart b/packages/mix/lib/src/core/style_builder.dart index 9efff57660..bed7c9c170 100644 --- a/packages/mix/lib/src/core/style_builder.dart +++ b/packages/mix/lib/src/core/style_builder.dart @@ -47,8 +47,11 @@ class StyleBuilder> extends StatefulWidget { } class _StyleBuilderState> extends State> { - /// State captured when an external controller is removed. The snapshot is - /// consumed only if the next build installs automatic pointer tracking. + /// A one-build state snapshot captured when an external controller is removed. + /// + /// It seeds a newly mounted [MixInteractionDetector] when automatic pointer + /// tracking is needed; otherwise it is discarded to avoid restoring stale + /// state later. Set? _controllerHandoffStates; Set _takeControllerHandoffStates() {