fix(mix): order variant merges by declared state dependencies

packages/mix/CHANGELOG.md

@@ -1,3 +1,20 @@
+## Unreleased
+
+### Fixes
+
+- **Variant merge priority follows declared state dependencies:** Priority now
+  groups active variants by whether they declare
+  `ContextVariant.widgetStateDependencies` rather than by whether they are a
+  `WidgetStateVariant`, so `onFocusVisible(...)` competes by declaration order
+  instead of always losing to any widget-state variant sharing a property, which
+  had been silently replacing focus rings. Variants built on
+  `ContextVariant.not(...)` move with their inner variant, so `onEnabled(...)`
+  now outranks an ambient variant such as `onDark(...)` declared after it.
+- **Declaration order within a priority group is reliable:** Grouping is a
+  stable partition rather than a `List.sort`, which fell back to an unstable
+  quicksort at 32 elements and could reorder equal-priority variants in styles
+  that large.
+
 ## 2.2.0-beta.3
 
 ### New features

packages/mix/lib/src/core/style.dart

@@ -102,43 +102,57 @@ abstract class Style<S extends Spec<S>> extends Mix<StyleSpec<S>>
 
   /// Merges all active variants with their nested variants recursively.
   ///
-  /// This method evaluates which variants should be active based on the current
-  /// context and named variants, then recursively processes nested variants
-  /// within each active variant's style. The result is a fully merged style
-  /// with all applicable variants applied.
+  /// Evaluates which variants are active in [context] and against
+  /// [namedVariants], then recursively resolves the nested variants inside each
+  /// active variant's style.
   ///
-  /// Variant priority order (lowest to highest):
-  /// 1. ContextVariant and NamedVariant (applied first)
-  /// 2. StyleVariation (applied second)
-  /// 3. WidgetStateVariant (applied last, highest priority)
+  /// Active variants apply in two priority groups, lowest first: those that
+  /// declare no [ContextVariant.widgetStateDependencies], then those that do.
+  /// Within a group, the variant declared last merges last and so wins on the
+  /// properties they share.
   @visibleForTesting
   Style<S> mergeActiveVariants(
     BuildContext context, {
     required Set<NamedVariant> namedVariants,
   }) {
-    // Filter variants that should be active in this context
-    final activeVariants = ($variants ?? [])
-        .where(
-          (variantAttr) => switch (variantAttr.variant) {
-            (ContextVariant variant) => variant.when(context),
-            (NamedVariant variant) => namedVariants.contains(variant),
-            (ContextVariantBuilder _) => true,
-          },
-        )
-        .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,
-      ),
-    );
+    final variants = $variants;
+    if (variants == null) return this;
+
+    // Partition, don't sort: declaration order inside a group is load-bearing,
+    // and List.sort is only stable by accident of the insertion sort it falls
+    // back to below 32 elements.
+    final lowPriority = <VariantStyle<S>>[];
+    final highPriority = <VariantStyle<S>>[];
+
+    for (final variantAttr in variants) {
+      final variant = variantAttr.variant;
+
+      final isActive = switch (variant) {
+        ContextVariant() => variant.when(context),
+        NamedVariant() => namedVariants.contains(variant),
+        ContextVariantBuilder() => true,
+      };
+      if (!isActive) continue;
+
+      // Keyed off the declaration, not the class: FocusVisibleVariant is not a
+      // WidgetStateVariant yet reads WidgetState.focused just the same, and
+      // NotVariant forwards whatever its inner variant reads.
+      //
+      // The getter stays on ContextVariant rather than moving up to Variant
+      // because ContextVariant is also the only kind widgetStates walks, so a
+      // dependency declared on any other kind would never get tracking
+      // installed.
+      final readsWidgetState =
+          variant is ContextVariant &&
+          variant.widgetStateDependencies.isNotEmpty;
+
+      (readsWidgetState ? highPriority : lowPriority).add(variantAttr);
+    }
 
     // Extract the style from each active variant
     final stylesToMerge = <(Style<S>, bool)>[]; // (style, isFromStyleVariation)
 
-    for (final variantAttr in activeVariants) {
+    for (final variantAttr in lowPriority.followedBy(highPriority)) {
       final result = switch (variantAttr.variant) {
         ContextVariantBuilder variant => (
           variant.build(context) as Style<S>,

packages/mix/test/src/core/style_get_all_variants_test.dart

@@ -219,6 +219,61 @@ void main() {
         final spec = result.resolve(context);
         expect((spec.resolvedValue as Map)['width'], 400.0);
       });
+
+      testWidgets(
+        'NotVariant inherits the priority of the variant it negates',
+        (tester) async {
+          final notHovered = ContextVariant.not(
+            ContextVariant.widgetState(WidgetState.hovered),
+          );
+          final ambient = ContextVariant('ambient', (context) => true);
+
+          final testAttribute = _MockSpecAttribute(
+            width: 50.0,
+            variants: [
+              // Declared first, but forwards hovered as a dependency, so it
+              // still applies after the ambient variant.
+              VariantStyle(notHovered, _MockSpecAttribute(width: 100.0)),
+              VariantStyle(ambient, _MockSpecAttribute(width: 200.0)),
+            ],
+          );
+
+          await testVariantPriority(
+            tester,
+            testAttribute: testAttribute,
+            activeStates: {},
+            namedVariants: {},
+            expectedWidth: 100.0,
+          );
+        },
+      );
+
+      testWidgets('declaration order holds past the 32-element sort threshold', (
+        tester,
+      ) async {
+        // List.sort switches from insertion sort to an unstable quicksort at 32
+        // elements, so this size is the one that would catch a regression back
+        // to sorting. All variants sit in the same priority group, so the last
+        // declared must win.
+        final testAttribute = _MockSpecAttribute(
+          width: 0.0,
+          variants: [
+            for (var i = 1; i <= 40; i++)
+              VariantStyle(
+                ContextVariant('context$i', (context) => true),
+                _MockSpecAttribute(width: i.toDouble()),
+              ),
+          ],
+        );
+
+        await testVariantPriority(
+          tester,
+          testAttribute: testAttribute,
+          activeStates: {},
+          namedVariants: {},
+          expectedWidth: 40.0,
+        );
+      });
     });
 
     group('Variant resolution logic', () {

packages/mix/test/src/variants/context_variant_test.dart

@@ -148,6 +148,37 @@ void main() {
       );
     });
 
+    testWidgets('onEnabled outranks an ambient variant declared after it', (
+      tester,
+    ) async {
+      // onEnabled is not(disabled), so it forwards a widget-state dependency
+      // and merges in the high-priority group. Pinned deliberately: it reads
+      // like base styling but does not behave like it.
+      await tester.pumpWidget(
+        MaterialApp(
+          home: MediaQuery(
+            data: const MediaQueryData(platformBrightness: Brightness.dark),
+            child: Box(
+              key: const Key('target'),
+              style: BoxStyler()
+                  .size(50, 50)
+                  .onEnabled(BoxStyler().color(Colors.blue))
+                  .onDark(BoxStyler().color(Colors.black)),
+            ),
+          ),
+        ),
+      );
+
+      final container = tester.widget<Container>(
+        find.descendant(
+          of: find.byKey(const Key('target')),
+          matching: find.byType(Container),
+        ),
+      );
+
+      expect((container.decoration as BoxDecoration?)?.color, Colors.blue);
+    });
+
     testWidgets('not variant inverts inner shouldApply result', (tester) async {
       final disabled = ContextVariant.widgetState(WidgetState.disabled);
       final enabled = ContextVariant.not(disabled);

packages/mix/test/src/variants/focus_visible_variant_test.dart

@@ -22,20 +22,27 @@ void main() {
       return (container.decoration as BoxDecoration?)?.color;
     }
 
-    Widget buildWithController(WidgetStatesController controller) {
+    Widget buildStyle(BoxStyler style, WidgetStatesController controller) {
       return MaterialApp(
         home: StyleBuilder<BoxSpec>(
           controller: controller,
-          style: BoxStyler()
-              .size(50, 50)
-              .color(Colors.blue)
-              .onFocusVisible(BoxStyler().color(Colors.red)),
+          style: style,
           builder: (context, spec) =>
               Container(key: const Key('target'), decoration: spec.decoration),
         ),
       );
     }
 
+    Widget buildWithController(WidgetStatesController controller) {
+      return buildStyle(
+        BoxStyler()
+            .size(50, 50)
+            .color(Colors.blue)
+            .onFocusVisible(BoxStyler().color(Colors.red)),
+        controller,
+      );
+    }
+
     testWidgets('tracks highlight mode without a Pressable ancestor', (
       tester,
     ) async {
@@ -110,5 +117,52 @@ void main() {
         Colors.red,
       );
     });
+
+    group('merge priority', () {
+      final selected = ContextVariant.widgetState(WidgetState.selected);
+
+      WidgetStatesController selectedAndFocusVisible() {
+        FocusManager.instance.highlightStrategy =
+            FocusHighlightStrategy.alwaysTraditional;
+        final controller = WidgetStatesController();
+        addTearDown(controller.dispose);
+        controller.selected = true;
+        controller.focused = true;
+
+        return controller;
+      }
+
+      testWidgets('beats a widget-state variant declared before it', (
+        tester,
+      ) async {
+        await tester.pumpWidget(
+          buildStyle(
+            BoxStyler()
+                .size(50, 50)
+                .variant(selected, BoxStyler().color(Colors.red))
+                .onFocusVisible(BoxStyler().color(Colors.blue)),
+            selectedAndFocusVisible(),
+          ),
+        );
+
+        expect(colorOf(tester), Colors.blue);
+      });
+
+      testWidgets('loses to a widget-state variant declared after it', (
+        tester,
+      ) async {
+        await tester.pumpWidget(
+          buildStyle(
+            BoxStyler()
+                .size(50, 50)
+                .onFocusVisible(BoxStyler().color(Colors.blue))
+                .variant(selected, BoxStyler().color(Colors.red)),
+            selectedAndFocusVisible(),
+          ),
+        );
+
+        expect(colorOf(tester), Colors.red);
+      });
+    });
   });
 }