diff --git a/packages/mix/CHANGELOG.md b/packages/mix/CHANGELOG.md index f80b49be6..bb0ae9718 100644 --- a/packages/mix/CHANGELOG.md +++ b/packages/mix/CHANGELOG.md @@ -1,3 +1,28 @@ +## 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:** `Style.widgetStates` now discovers state + requirements recursively through nested and negated variants with + 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.2 ### New features 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 712fe81fd..3ad3b6ef3 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 fd619c1a6..7d046657d 100644 --- a/packages/mix/lib/src/core/style.dart +++ b/packages/mix/lib/src/core/style.dart @@ -66,10 +66,34 @@ 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 = {}; + // 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; + + 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/core/style_builder.dart b/packages/mix/lib/src/core/style_builder.dart index ca5fab0ee..5fcc9e559 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 67150f42b..a0c6ad17e 100644 --- a/packages/mix/lib/src/variants/variant.dart +++ b/packages/mix/lib/src/variants/variant.dart @@ -108,6 +108,21 @@ class ContextVariant extends Variant { return ContextVariant.breakpoint(BreakpointToken.desktop()); } + /// 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 bool when(BuildContext context) { return shouldApply(context); @@ -185,6 +200,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 +272,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 983c9cd11..69315e0a6 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,61 @@ 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, reason: 'before hover'); + + 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, 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(); + } + testWidgets('hover variant changes style when mouse enters and exits', ( tester, ) async { @@ -178,5 +233,326 @@ 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, + ); + }); + + 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); + 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}); + }); + }); + + 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', + ); + }); + }); }); } 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 fc87a0c5d..3cfbef995 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', () {