From 3bbb4d62fa866306f9d7daf123c5333af087cf5d Mon Sep 17 00:00:00 2001 From: Leo Farias Date: Sat, 11 Jul 2026 11:02:49 -0400 Subject: [PATCH 1/3] fix(mix): honor modifier order and interpolation endpoints MixScope.orderOfModifiers was accepted and inherited but never read by resolve(), so the setting had no rendering effect. ModifierListTween also iterated only the target list, so begin-only modifiers vanished immediately and target-only modifiers without a registered neutral default were present before t=0.5 but absent at t=1, breaking lerp(1) == end. Establish explicit precedence (style-local > nearest MixScope > package default > remaining types), subscribe to the modifierOrder aspect only when it can change the result, and rewrite ModifierListTween.lerp to interpolate over the union of begin/end identities with exact endpoint returns. --- .../src/modifiers/widget_modifier_config.dart | 199 +++++++++----- .../modifiers/modifier_list_tween_test.dart | 230 +++++++++++++++++ .../modifier_order_precedence_test.dart | 243 ++++++++++++++++++ 3 files changed, 610 insertions(+), 62 deletions(-) create mode 100644 packages/mix/test/src/modifiers/modifier_list_tween_test.dart create mode 100644 packages/mix/test/src/modifiers/modifier_order_precedence_test.dart diff --git a/packages/mix/lib/src/modifiers/widget_modifier_config.dart b/packages/mix/lib/src/modifiers/widget_modifier_config.dart index 4263c68a6..e6c3f56ef 100644 --- a/packages/mix/lib/src/modifiers/widget_modifier_config.dart +++ b/packages/mix/lib/src/modifiers/widget_modifier_config.dart @@ -10,6 +10,7 @@ import '../properties/typography/text_style_mix.dart'; import '../specs/box/box_spec.dart'; import '../specs/icon/icon_spec.dart'; import '../specs/text/text_spec.dart'; +import '../theme/mix_theme.dart'; import 'align_modifier.dart'; import 'aspect_ratio_modifier.dart'; import 'blur_modifier.dart'; @@ -355,33 +356,17 @@ final class WidgetModifierConfig with Equatable { ); } - /// Orders modifiers according to the specified order or default order. + /// Orders modifiers according to the style-local order, then the package + /// default order, then any remaining types. + /// + /// This ignores the ambient [MixScope] order; [resolve] layers that in with + /// the correct precedence. Kept for its `@visibleForTesting` surface. @visibleForTesting List reorderModifiers(List modifiers) { - if (modifiers.isEmpty) return modifiers; - - final orderOfModifiers = { - // Prioritize the order of modifiers provided by the user. - ...?$orderOfModifiers, - // Add the default order of modifiers. - ..._defaultOrder, - // Add any remaining modifiers that were not included in the order. - ...modifiers.map((e) => e.runtimeType), - }.toList(); - - final orderedSpecs = []; - - for (final modifierType in orderOfModifiers) { - // Find and add modifiers matching this type - final modifier = modifiers - .where((e) => e.runtimeType == modifierType) - .firstOrNull; - if (modifier != null) { - orderedSpecs.add(modifier); - } - } - - return orderedSpecs; + return _orderModifiers( + modifiers, + preferredOrder: $orderOfModifiers ?? const [], + ); } WidgetModifierConfig shaderMask({ @@ -610,27 +595,88 @@ final class WidgetModifierConfig with Equatable { /// Resolves the modifiers into a properly ordered list ready for rendering. /// - /// It's important to order the list before resolving to ensure the correct order of modifiers. + /// The resolved list is ordered outermost-to-innermost with this precedence: + /// + /// style-local order > nearest MixScope order > package default order + /// > remaining configured types in stable order List resolve(BuildContext context) { - if ($modifiers == null || $modifiers!.isEmpty) return []; + if ($modifiers == null || $modifiers!.isEmpty) return const []; - // Resolve each modifier attribute to its corresponding modifier spec + // Resolve each modifier attribute to its corresponding modifier spec. + // Reset specs are filtered here so they never reach rendering. final resolvedModifiers = []; for (final attribute in $modifiers!) { final resolved = attribute.resolve(context); - // Filter out reset specs so they never reach rendering if (resolved is! ResetModifier) { resolvedModifiers.add(resolved as WidgetModifier); } } + if (resolvedModifiers.isEmpty) return const []; - return reorderModifiers(resolvedModifiers); + // Only subscribe to the `modifierOrder` inherited-model aspect when a scope + // order could actually change the result: modifiers are present and the + // style-local order does not already cover every present type. This keeps a + // modifier-order-only consumer from rebuilding on unrelated token changes, + // and avoids a spurious dependency when the local order fully determines + // the outcome. + final scopeOrder = _localOrderFullyCovers(resolvedModifiers) + ? null + : MixScope.maybeOf(context, 'modifierOrder')?.orderOfModifiers; + + return _orderModifiers( + resolvedModifiers, + preferredOrder: [...?$orderOfModifiers, ...?scopeOrder], + ); + } + + /// Whether the style-local [$orderOfModifiers] already lists every present + /// modifier type, in which case the scope order cannot affect the outcome. + bool _localOrderFullyCovers(List modifiers) { + final local = $orderOfModifiers; + if (local == null || local.isEmpty) return false; + final localTypes = local.toSet(); + + return modifiers.every((m) => localTypes.contains(m.runtimeType)); } @override List get props => [$orderOfModifiers, $modifiers]; } +/// Orders [modifiers] outermost-to-innermost by type. +/// +/// [preferredOrder] wins first (the caller concatenates style-local order then +/// nearest scope order), then the package [_defaultOrder], then any remaining +/// configured types in stable (first-seen) order. Each type is emitted once. +/// +/// Shared by [WidgetModifierConfig.resolve]/[WidgetModifierConfig.reorderModifiers] +/// and [ModifierListTween] so animated frames nest consistently with resolved +/// lists. +List _orderModifiers( + List modifiers, { + List preferredOrder = const [], +}) { + if (modifiers.isEmpty) return modifiers; + + final orderOfModifiers = { + ...preferredOrder, + ..._defaultOrder, + ...modifiers.map((e) => e.runtimeType), + }; + + final orderedSpecs = []; + for (final modifierType in orderOfModifiers) { + final modifier = modifiers + .where((e) => e.runtimeType == modifierType) + .firstOrNull; + if (modifier != null) { + orderedSpecs.add(modifier); + } + } + + return orderedSpecs; +} + const _defaultOrder = [ // === PHASE 1: CONTEXT & BEHAVIOR SETUP === @@ -721,6 +767,18 @@ const _defaultOrder = [ ShaderMaskModifier, ]; +/// Neutral (identity) values used as the interpolation anchor when a modifier +/// type is present on only one side of a [ModifierListTween]. +/// +/// Each entry's default constructor is a genuine no-op: `OpacityModifier()` is +/// opacity `1.0`, `PaddingModifier()` is `EdgeInsets.zero`, `TransformModifier()` +/// is `Matrix4.identity()`, `BlurModifier()` is sigma `0.0`, and so on — so an +/// addition/removal fades through "no visible effect". +/// +/// Types with no genuine neutral are deliberately omitted so the tween falls +/// back to the documented midpoint snap instead of inventing a fake anchor: +/// `ShaderMaskModifier` (arbitrary shader), `MouseCursorModifier`, +/// `ScrollViewModifier`, `BoxModifier`, and `DefaultTextStylerModifier`. final defaultModifier = { FlexibleModifier: FlexibleModifier(), VisibilityModifier: VisibilityModifier(), @@ -752,42 +810,59 @@ class ModifierListTween extends Tween?> { @override List? lerp(double t) { - List? lerpedModifiers; - if (end != null) { - // Use empty list if begin is null (handles lerp from null to non-null) - final thisModifiers = begin ?? []; - final otherModifiers = end!; - - // Create a map of modifiers by runtime type from the other list - final thisModifierMap = {}; - - for (final modifier in thisModifiers) { - thisModifierMap[modifier.runtimeType] = modifier; + // Exact endpoints first. Guarantees lerp(0) == begin and lerp(1) == end for + // every membership case, before any fallback/floating-point logic can lose + // begin/end identity. Exact equality (not <=/>=) is deliberate so curves + // that overshoot the [0, 1] range still extrapolate modifier values. + if (t == 0.0) return begin; + if (t == 1.0) return end; + + final beginModifiers = begin ?? const []; + final endModifiers = end ?? const []; + + final beginByType = { + for (final m in beginModifiers) m.runtimeType: m, + }; + final endByType = { + for (final m in endModifiers) m.runtimeType: m, + }; + + // Interpolate over the union of begin and target identities. Runtime type + // is the matching identity (multiple instances of one type is out of scope). + final lerped = []; + + // Target members, in target order. + for (final endModifier in endModifiers) { + final from = + beginByType[endModifier.runtimeType] ?? + defaultModifier[endModifier.runtimeType] as WidgetModifier?; + if (from != null) { + // Present in both, or target-only with a neutral default: + // interpolate begin/neutral -> target. + lerped.add(from.lerp(endModifier, t) as WidgetModifier); + } else if (t >= 0.5) { + // Target-only without a neutral default: snap in at the midpoint. + lerped.add(endModifier); } + } - // Lerp each modifier from this list with its matching type from other - lerpedModifiers = []; - for (final modifier in otherModifiers) { - WidgetModifier? thisModifier = thisModifierMap[modifier.runtimeType]; - thisModifier ??= - defaultModifier[modifier.runtimeType] as WidgetModifier?; - - if (thisModifier != null) { - // Both have this modifier type, lerp them - // We need to use dynamic dispatch here since lerp is type-specific - final lerpedModifier = - thisModifier.lerp(modifier, t) as WidgetModifier; - lerpedModifiers.add(lerpedModifier); - } else { - // Only this has the modifier, fade it out if t > 0.5 - - if (t < 0.5) { - lerpedModifiers.add(modifier); - } - } + // Beginning-only members, in beginning order, until they disappear. + for (final beginModifier in beginModifiers) { + if (endByType.containsKey(beginModifier.runtimeType)) continue; + final to = defaultModifier[beginModifier.runtimeType] as WidgetModifier?; + if (to != null) { + // Beginning-only with a neutral default: interpolate begin -> neutral. + lerped.add(beginModifier.lerp(to, t) as WidgetModifier); + } else if (t < 0.5) { + // Beginning-only without a neutral default: snap out at the midpoint. + lerped.add(beginModifier); } } - return lerpedModifiers; + if (lerped.isEmpty) return null; + + // Order the transient union with the same precedence as resolved lists so + // intermediate frames nest consistently with the begin/end endpoints. + return _orderModifiers(lerped); } } diff --git a/packages/mix/test/src/modifiers/modifier_list_tween_test.dart b/packages/mix/test/src/modifiers/modifier_list_tween_test.dart new file mode 100644 index 000000000..810d08a12 --- /dev/null +++ b/packages/mix/test/src/modifiers/modifier_list_tween_test.dart @@ -0,0 +1,230 @@ +import 'package:flutter/material.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:mix/mix.dart'; + +/// A custom [WidgetModifier] that is intentionally absent from `defaultModifier`, +/// so it exercises the "no neutral default" interpolation path with a type that +/// is not a built-in. +final class _TagModifier extends WidgetModifier<_TagModifier> { + const _TagModifier(this.tag); + + final String tag; + + @override + _TagModifier copyWith() => _TagModifier(tag); + + @override + _TagModifier lerp(_TagModifier? other, double t) => + t < 0.5 ? this : (other ?? this); + + @override + List get props => [tag]; + + @override + Widget build(Widget child) => + KeyedSubtree(key: ValueKey('tag:$tag'), child: child); +} + +/// Returns the first modifier of type [X] in [list], or null if absent. +X? _firstOfType>(List? list) { + if (list == null) return null; + for (final m in list) { + if (m is X) return m; + } + + return null; +} + +bool _has>(List? list) => + _firstOfType(list) != null; + +void main() { + // Membership snap point documented by ModifierListTween. + const snap = 0.5; + const belowSnap = 0.49; + + group('ModifierListTween.lerp — endpoint invariants', () { + // lerp(0) == begin and lerp(1) == end for every membership case in the + // interpolation matrix. + final cases = + ? begin, List? end})>{ + 'present -> present (neutral yes)': ( + begin: [OpacityModifier(0.2)], + end: [OpacityModifier(0.8)], + ), + 'present -> present (neutral no)': ( + begin: [MouseCursorModifier(mouseCursor: SystemMouseCursors.click)], + end: [ + MouseCursorModifier(mouseCursor: SystemMouseCursors.forbidden), + ], + ), + 'absent -> present (neutral yes)': ( + begin: [], + end: [OpacityModifier(0.8)], + ), + 'absent -> present (neutral no)': ( + begin: [], + end: [MouseCursorModifier(mouseCursor: SystemMouseCursors.click)], + ), + 'present -> absent (neutral yes)': ( + begin: [PaddingModifier(const EdgeInsets.all(10))], + end: [], + ), + 'present -> absent (neutral no)': ( + begin: [MouseCursorModifier(mouseCursor: SystemMouseCursors.click)], + end: [], + ), + 'custom absent -> present (no neutral)': ( + begin: [], + end: [const _TagModifier('x')], + ), + 'custom present -> absent (no neutral)': ( + begin: [const _TagModifier('x')], + end: [], + ), + 'null begin': (begin: null, end: [OpacityModifier(0.8)]), + 'null end': (begin: [OpacityModifier(0.2)], end: null), + }; + + cases.forEach((name, data) { + test('$name: lerp(0) == begin, lerp(1) == end', () { + final tween = ModifierListTween(begin: data.begin, end: data.end); + + expect(tween.lerp(0.0), data.begin, reason: 'lerp(0) must equal begin'); + expect(tween.lerp(1.0), data.end, reason: 'lerp(1) must equal end'); + }); + }); + }); + + group('ModifierListTween.lerp — issue probe regression', () { + test('addition without a neutral is absent at t=0 and present at t=1', () { + final tween = ModifierListTween( + begin: const [], + end: [MouseCursorModifier(mouseCursor: SystemMouseCursors.click)], + ); + + // On origin/main this was inverted: present at t=0, empty at t=1. + expect(_has(tween.lerp(0.0)), isFalse); + expect( + _firstOfType(tween.lerp(1.0))?.mouseCursor, + SystemMouseCursors.click, + ); + }); + }); + + group('ModifierListTween.lerp — present on both sides', () { + test('interpolates the value across the whole range', () { + final tween = ModifierListTween( + begin: [OpacityModifier(0.2)], + end: [OpacityModifier(0.8)], + ); + + expect( + _firstOfType(tween.lerp(0.5))!.opacity, + closeTo(0.5, 1e-9), + ); + expect(_has(tween.lerp(belowSnap)), isTrue); + expect(_has(tween.lerp(snap)), isTrue); + }); + }); + + group( + 'ModifierListTween.lerp — addition with a neutral default (opacity)', + () { + final tween = ModifierListTween( + begin: const [], + end: [OpacityModifier(0.8)], + ); + + test('present throughout, interpolating neutral(1.0) -> target(0.8)', () { + // Present before, at, and after the snap point. + expect(_has(tween.lerp(belowSnap)), isTrue); + expect(_has(tween.lerp(snap)), isTrue); + expect(_has(tween.lerp(0.9)), isTrue); + + // Neutral is 1.0, so at t=0.5 it is halfway to 0.8. + expect( + _firstOfType(tween.lerp(0.5))!.opacity, + closeTo(0.9, 1e-9), + ); + }); + }, + ); + + group( + 'ModifierListTween.lerp — removal with a neutral default (padding)', + () { + final tween = ModifierListTween( + begin: [PaddingModifier(const EdgeInsets.all(10))], + end: const [], + ); + + test('present throughout, interpolating begin(10) -> neutral(0)', () { + expect(_has(tween.lerp(belowSnap)), isTrue); + expect(_has(tween.lerp(snap)), isTrue); + expect(_has(tween.lerp(0.9)), isTrue); + + expect( + _firstOfType(tween.lerp(0.5))!.padding, + const EdgeInsets.all(5), + ); + }); + }, + ); + + group('ModifierListTween.lerp — addition without a neutral (snap in)', () { + // Uses both a built-in (MouseCursor) and a custom modifier (_TagModifier). + test('absent before the snap point, present at/after it', () { + final cursor = ModifierListTween( + begin: const [], + end: [MouseCursorModifier(mouseCursor: SystemMouseCursors.click)], + ); + expect(_has(cursor.lerp(belowSnap)), isFalse); + expect(_has(cursor.lerp(snap)), isTrue); + expect(_has(cursor.lerp(0.9)), isTrue); + + final custom = ModifierListTween( + begin: const [], + end: [const _TagModifier('x')], + ); + expect(_has<_TagModifier>(custom.lerp(belowSnap)), isFalse); + expect(_has<_TagModifier>(custom.lerp(snap)), isTrue); + }); + }); + + group('ModifierListTween.lerp — removal without a neutral (snap out)', () { + test('present before the snap point, absent at/after it', () { + final cursor = ModifierListTween( + begin: [MouseCursorModifier(mouseCursor: SystemMouseCursors.click)], + end: const [], + ); + expect(_has(cursor.lerp(belowSnap)), isTrue); + expect(_has(cursor.lerp(snap)), isFalse); + expect(_has(cursor.lerp(0.9)), isFalse); + + final custom = ModifierListTween( + begin: [const _TagModifier('x')], + end: const [], + ); + expect(_has<_TagModifier>(custom.lerp(belowSnap)), isTrue); + expect(_has<_TagModifier>(custom.lerp(snap)), isFalse); + }); + }); + + group('ModifierListTween.lerp — mixed membership keeps target order', () { + test('union is ordered by the canonical modifier order', () { + // Opacity is outermost-last in the default order; Padding precedes it. + final tween = ModifierListTween( + begin: [OpacityModifier(0.2)], + end: [PaddingModifier(const EdgeInsets.all(8)), OpacityModifier(0.8)], + ); + + final mid = tween.lerp(0.5)!; + final types = mid.map((m) => m.runtimeType).toList(); + expect( + types.indexOf(PaddingModifier), + lessThan(types.indexOf(OpacityModifier)), + ); + }); + }); +} diff --git a/packages/mix/test/src/modifiers/modifier_order_precedence_test.dart b/packages/mix/test/src/modifiers/modifier_order_precedence_test.dart new file mode 100644 index 000000000..37f5af6ff --- /dev/null +++ b/packages/mix/test/src/modifiers/modifier_order_precedence_test.dart @@ -0,0 +1,243 @@ +import 'package:flutter/material.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:mix/mix.dart'; + +import '../../helpers/testing_utils.dart'; + +/// Two built-ins whose package default order is Padding-outside-Opacity, so a +/// scope or style-local order of `[Opacity, Padding]` is observably different +/// from the default. +WidgetModifierConfig _opacityThenPadding() => WidgetModifierConfig.modifiers([ + OpacityModifierMix(opacity: 0.5), + PaddingModifierMix(padding: EdgeInsetsGeometryMix.all(10)), +]); + +void main() { + group('Modifier order precedence (resolved list)', () { + test('package default places Padding outside Opacity', () { + final types = _opacityThenPadding() + .resolve(MockBuildContext()) + .map((m) => m.runtimeType) + .toList(); + + expect(types, [PaddingModifier, OpacityModifier]); + }); + + test('nearest MixScope order overrides the package default', () { + final types = _opacityThenPadding() + .resolve( + MockBuildContext( + orderOfModifiers: const [OpacityModifier, PaddingModifier], + ), + ) + .map((m) => m.runtimeType) + .toList(); + + expect(types, [OpacityModifier, PaddingModifier]); + }); + + test('style-local order wins over MixScope order', () { + final config = _opacityThenPadding().orderOfModifiers(const [ + PaddingModifier, + OpacityModifier, + ]); + + final types = config + .resolve( + MockBuildContext( + // Scope asks for the opposite order; local must win. + orderOfModifiers: const [OpacityModifier, PaddingModifier], + ), + ) + .map((m) => m.runtimeType) + .toList(); + + expect(types, [PaddingModifier, OpacityModifier]); + }); + + test('types absent from every order stay stable and appear once', () { + final config = WidgetModifierConfig.modifiers([ + MouseCursorModifierMix(mouseCursor: SystemMouseCursors.click), + ScrollViewModifierMix(), + OpacityModifierMix(opacity: 0.5), + ]); + + final types = config + .resolve(MockBuildContext()) + .map((m) => m.runtimeType) + .toList(); + + // Each unlisted type appears exactly once. + expect(types.where((t) => t == MouseCursorModifier).length, 1); + expect(types.where((t) => t == ScrollViewModifier).length, 1); + // A known type is placed by the default order, ahead of unlisted types. + expect( + types.indexOf(OpacityModifier), + lessThan(types.indexOf(MouseCursorModifier)), + ); + // Unlisted types keep their configured (first-seen) order. + expect( + types.indexOf(MouseCursorModifier), + lessThan(types.indexOf(ScrollViewModifier)), + ); + }); + }); + + group('Modifier order precedence (rendered wrapper tree)', () { + // list[0] is the outermost wrapper, so it is the ancestor in the tree. + final opacity = find.byWidgetPredicate( + (w) => w is Opacity && w.opacity == 0.5, + ); + final padding = find.byWidgetPredicate( + (w) => w is Padding && w.padding == const EdgeInsets.all(10), + ); + + Widget build({List? scopeOrder, List? localOrder}) { + var config = _opacityThenPadding(); + if (localOrder != null) config = config.orderOfModifiers(localOrder); + + final consumer = StyleBuilder( + style: BoxStyler() + .width(100) + .height(100) + .color(Colors.blue) + .wrap(config), + builder: (context, spec) => Container( + decoration: spec.decoration, + constraints: spec.constraints, + ), + ); + + return MaterialApp( + home: scopeOrder == null + ? MixScope.empty(child: consumer) + : MixScope(orderOfModifiers: scopeOrder, child: consumer), + ); + } + + testWidgets('default order nests Padding outside Opacity', (tester) async { + await tester.pumpWidget(build()); + + expect(find.descendant(of: padding, matching: opacity), findsOneWidget); + expect(find.descendant(of: opacity, matching: padding), findsNothing); + }); + + testWidgets('MixScope order changes the actual widget nesting', ( + tester, + ) async { + await tester.pumpWidget( + build(scopeOrder: const [OpacityModifier, PaddingModifier]), + ); + + expect(find.descendant(of: opacity, matching: padding), findsOneWidget); + expect(find.descendant(of: padding, matching: opacity), findsNothing); + }); + + testWidgets('style-local order overrides scope order in the tree', ( + tester, + ) async { + await tester.pumpWidget( + build( + scopeOrder: const [OpacityModifier, PaddingModifier], + localOrder: const [PaddingModifier, OpacityModifier], + ), + ); + + expect(find.descendant(of: padding, matching: opacity), findsOneWidget); + expect(find.descendant(of: opacity, matching: padding), findsNothing); + }); + }); + + group('Modifier order aspect precision', () { + testWidgets( + 'token-only change does not re-resolve; scope order change does', + (tester) async { + var buildCount = 0; + + // A modifier-order-only consumer: has modifiers, no style-local order, + // and uses no tokens — so its only scope dependency is `modifierOrder`. + final consumer = StyleBuilder( + style: BoxStyler() + .width(100) + .height(100) + .color(Colors.blue) + .wrap(_opacityThenPadding()), + builder: (context, spec) { + buildCount++; + + return Container( + decoration: spec.decoration, + constraints: spec.constraints, + ); + }, + ); + + final host = GlobalKey<_ScopeHostState>(); + await tester.pumpWidget( + MaterialApp( + home: _ScopeHost(key: host, child: consumer), + ), + ); + await tester.pump(); + + final afterMount = buildCount; + expect(afterMount, greaterThan(0)); + + // Token-only change must not invalidate a modifier-order-only consumer. + host.currentState!.update( + colors: {const ColorToken('brand'): Colors.red}, + ); + await tester.pump(); + expect( + buildCount, + afterMount, + reason: 'token-only change must not re-resolve the consumer', + ); + + // Scope modifier-order change must re-resolve the consumer. + host.currentState!.update( + order: const [OpacityModifier, PaddingModifier], + ); + await tester.pump(); + expect( + buildCount, + greaterThan(afterMount), + reason: 'scope order change must re-resolve the consumer', + ); + }, + ); + }); +} + +/// Test host that keeps its [child] instance stable across rebuilds so that +/// changing only the [MixScope]'s tokens/order exercises the inherited-model +/// aspect filter instead of structurally rebuilding the subtree. +class _ScopeHost extends StatefulWidget { + const _ScopeHost({super.key, required this.child}); + + final Widget child; + + @override + State<_ScopeHost> createState() => _ScopeHostState(); +} + +class _ScopeHostState extends State<_ScopeHost> { + Map? _colors; + List? _order; + + void update({Map? colors, List? order}) { + setState(() { + if (colors != null) _colors = colors; + if (order != null) _order = order; + }); + } + + @override + Widget build(BuildContext context) { + return MixScope( + colors: _colors, + orderOfModifiers: _order, + child: widget.child, + ); + } +} From 435a2da8005088abd705e41ef5f0f369f1261e40 Mon Sep 17 00:00:00 2001 From: Leo Farias Date: Sat, 11 Jul 2026 12:45:21 -0400 Subject: [PATCH 2/3] fix(mix): preserve modifier animation semantics --- .../animation/style_animation_builder.dart | 27 +++++-- .../src/animation/style_animation_driver.dart | 27 +++++-- .../src/modifiers/widget_modifier_config.dart | 55 +++++--------- .../modifiers/modifier_list_tween_test.dart | 59 ++++++++++++++ .../modifier_order_precedence_test.dart | 76 +++++++++++++++++++ 5 files changed, 192 insertions(+), 52 deletions(-) diff --git a/packages/mix/lib/src/animation/style_animation_builder.dart b/packages/mix/lib/src/animation/style_animation_builder.dart index 5a833d5cc..522644d85 100644 --- a/packages/mix/lib/src/animation/style_animation_builder.dart +++ b/packages/mix/lib/src/animation/style_animation_builder.dart @@ -33,14 +33,7 @@ class _StyleAnimationBuilderState> extends State> with TickerProviderStateMixin { late StyleAnimationDriver animationDriver; - - @override - void initState() { - super.initState(); - final spec = widget.spec; - final config = spec.animation; - animationDriver = _createAnimationDriver(config: config, initialSpec: spec); - } + bool _hasInitializedAnimationDriver = false; StyleAnimationDriver _createAnimationDriver({ required AnimationConfig? config, @@ -78,6 +71,24 @@ class _StyleAnimationBuilderState> }; } + @override + void didChangeDependencies() { + super.didChangeDependencies(); + + if (!_hasInitializedAnimationDriver) { + final spec = widget.spec; + animationDriver = _createAnimationDriver( + config: spec.animation, + initialSpec: spec, + ); + _hasInitializedAnimationDriver = true; + + return; + } + + animationDriver.didChangeDependencies(); + } + @override void dispose() { animationDriver.dispose(); diff --git a/packages/mix/lib/src/animation/style_animation_driver.dart b/packages/mix/lib/src/animation/style_animation_driver.dart index 578e43054..ec6498bb3 100644 --- a/packages/mix/lib/src/animation/style_animation_driver.dart +++ b/packages/mix/lib/src/animation/style_animation_driver.dart @@ -53,6 +53,8 @@ abstract class StyleAnimationDriver> { // ignore: no-empty-block void didUpdateSpec(StyleSpec oldSpec, StyleSpec newSpec) {} + // ignore: no-empty-block + void didChangeDependencies() {} void updateDriver(AnimationConfig config); /// Execute the animation (curve vs spring). @@ -245,14 +247,7 @@ class PhaseAnimationDriver> extends StyleAnimationDriver { } void _setUpAnimation() { - final specs = config.styles - .map((e) => e.resolve(context) as StyleSpec) - .toList(); - - _tweenSequence = _createTweenSequence(specs, config.curveConfigs); - - // Override the animation to use TweenSequence wrapped in a tween - _animation = controller.drive(_PhasedSpecTween(_tweenSequence)); + _resolveStyles(); config.trigger?.addListener(_onTriggerChanged); @@ -266,6 +261,17 @@ class PhaseAnimationDriver> extends StyleAnimationDriver { } } + void _resolveStyles() { + final specs = config.styles + .map((e) => e.resolve(context) as StyleSpec) + .toList(); + + _tweenSequence = _createTweenSequence(specs, config.curveConfigs); + + // Override the animation to use TweenSequence wrapped in a tween + _animation = controller.drive(_PhasedSpecTween(_tweenSequence)); + } + void _onTriggerChanged() { executeAnimation(); } @@ -331,6 +337,11 @@ class PhaseAnimationDriver> extends StyleAnimationDriver { await controller.forward(from: 0.0); } + @override + void didChangeDependencies() { + _resolveStyles(); + } + @override void updateDriver(covariant PhaseAnimationConfig config) { this.config.trigger?.removeListener(_onTriggerChanged); diff --git a/packages/mix/lib/src/modifiers/widget_modifier_config.dart b/packages/mix/lib/src/modifiers/widget_modifier_config.dart index e6c3f56ef..5706f53f9 100644 --- a/packages/mix/lib/src/modifiers/widget_modifier_config.dart +++ b/packages/mix/lib/src/modifiers/widget_modifier_config.dart @@ -343,6 +343,16 @@ final class WidgetModifierConfig with Equatable { } } + /// Whether the style-local [$orderOfModifiers] already lists every present + /// modifier type, in which case the scope order cannot affect the outcome. + bool _localOrderFullyCovers(List modifiers) { + final local = $orderOfModifiers; + if (local == null || local.isEmpty) return false; + final localTypes = local.toSet(); + + return modifiers.every((m) => localTypes.contains(m.runtimeType)); + } + WidgetModifierConfig translate({required double x, required double y}) { return merge(WidgetModifierConfig.translate(x: x, y: y)); } @@ -629,16 +639,6 @@ final class WidgetModifierConfig with Equatable { ); } - /// Whether the style-local [$orderOfModifiers] already lists every present - /// modifier type, in which case the scope order cannot affect the outcome. - bool _localOrderFullyCovers(List modifiers) { - final local = $orderOfModifiers; - if (local == null || local.isEmpty) return false; - final localTypes = local.toSet(); - - return modifiers.every((m) => localTypes.contains(m.runtimeType)); - } - @override List get props => [$orderOfModifiers, $modifiers]; } @@ -649,9 +649,8 @@ final class WidgetModifierConfig with Equatable { /// nearest scope order), then the package [_defaultOrder], then any remaining /// configured types in stable (first-seen) order. Each type is emitted once. /// -/// Shared by [WidgetModifierConfig.resolve]/[WidgetModifierConfig.reorderModifiers] -/// and [ModifierListTween] so animated frames nest consistently with resolved -/// lists. +/// Shared by [WidgetModifierConfig.resolve] and +/// [WidgetModifierConfig.reorderModifiers]. List _orderModifiers( List modifiers, { List preferredOrder = const [], @@ -770,37 +769,23 @@ const _defaultOrder = [ /// Neutral (identity) values used as the interpolation anchor when a modifier /// type is present on only one side of a [ModifierListTween]. /// -/// Each entry's default constructor is a genuine no-op: `OpacityModifier()` is -/// opacity `1.0`, `PaddingModifier()` is `EdgeInsets.zero`, `TransformModifier()` -/// is `Matrix4.identity()`, `BlurModifier()` is sigma `0.0`, and so on — so an -/// addition/removal fades through "no visible effect". +/// Each entry can interpolate through its default constructor without changing +/// layout, paint, inherited data, or clipping. For example, +/// `OpacityModifier()` is opacity `1.0`, `PaddingModifier()` is +/// `EdgeInsets.zero`, `TransformModifier()` is `Matrix4.identity()`, and +/// `BlurModifier()` is sigma `0.0`. /// /// Types with no genuine neutral are deliberately omitted so the tween falls -/// back to the documented midpoint snap instead of inventing a fake anchor: -/// `ShaderMaskModifier` (arbitrary shader), `MouseCursorModifier`, -/// `ScrollViewModifier`, `BoxModifier`, and `DefaultTextStylerModifier`. +/// back to the documented midpoint snap instead of inventing a fake anchor. final defaultModifier = { - FlexibleModifier: FlexibleModifier(), VisibilityModifier: VisibilityModifier(), - IconThemeModifier: IconThemeModifier(), - DefaultTextStyleModifier: DefaultTextStyleModifier(), - SizedBoxModifier: SizedBoxModifier(), - FractionallySizedBoxModifier: FractionallySizedBoxModifier(), - IntrinsicHeightModifier: IntrinsicHeightModifier(), - IntrinsicWidthModifier: IntrinsicWidthModifier(), - AspectRatioModifier: AspectRatioModifier(), RotatedBoxModifier: RotatedBoxModifier(), - AlignModifier: AlignModifier(), PaddingModifier: PaddingModifier(), TransformModifier: TransformModifier(), ScaleModifier: ScaleModifier(), RotateModifier: RotateModifier(), TranslateModifier: TranslateModifier(), SkewModifier: SkewModifier(), - ClipOvalModifier: ClipOvalModifier(), - ClipRRectModifier: ClipRRectModifier(), - ClipPathModifier: ClipPathModifier(), - ClipTriangleModifier: ClipTriangleModifier(), BlurModifier: BlurModifier(), OpacityModifier: OpacityModifier(), }; @@ -861,8 +846,6 @@ class ModifierListTween extends Tween?> { if (lerped.isEmpty) return null; - // Order the transient union with the same precedence as resolved lists so - // intermediate frames nest consistently with the begin/end endpoints. - return _orderModifiers(lerped); + return lerped; } } diff --git a/packages/mix/test/src/modifiers/modifier_list_tween_test.dart b/packages/mix/test/src/modifiers/modifier_list_tween_test.dart index 810d08a12..dc9022a46 100644 --- a/packages/mix/test/src/modifiers/modifier_list_tween_test.dart +++ b/packages/mix/test/src/modifiers/modifier_list_tween_test.dart @@ -209,6 +209,53 @@ void main() { expect(_has<_TagModifier>(custom.lerp(belowSnap)), isTrue); expect(_has<_TagModifier>(custom.lerp(snap)), isFalse); }); + + // Null dimensions/factors make SizedBox and FractionallySizedBox no-ops + // when built, but their numeric lerps treat null as zero. They therefore + // cannot serve as interpolation identities either. + final modifiersWithoutIdentity = [ + const FlexibleModifier(), + const IconThemeModifier(), + const DefaultTextStyleModifier(), + const SizedBoxModifier(), + const FractionallySizedBoxModifier(), + const IntrinsicHeightModifier(), + const IntrinsicWidthModifier(), + const AspectRatioModifier(), + const AlignModifier(), + const ClipOvalModifier(), + const ClipRRectModifier(), + const ClipPathModifier(), + const ClipTriangleModifier(), + ]; + + for (final modifier in modifiersWithoutIdentity) { + test('${modifier.runtimeType} snaps in both directions', () { + final addition = ModifierListTween(begin: const [], end: [modifier]); + final removal = ModifierListTween(begin: [modifier], end: const []); + + expect( + addition.lerp(belowSnap), + isNull, + reason: '${modifier.runtimeType} has no neutral addition anchor', + ); + expect( + addition.lerp(snap)?.single.runtimeType, + modifier.runtimeType, + reason: '${modifier.runtimeType} must snap in at the midpoint', + ); + expect( + removal.lerp(belowSnap)?.single.runtimeType, + modifier.runtimeType, + reason: '${modifier.runtimeType} must remain before the midpoint', + ); + expect( + removal.lerp(snap), + isNull, + reason: '${modifier.runtimeType} has no neutral removal anchor', + ); + }); + } }); group('ModifierListTween.lerp — mixed membership keeps target order', () { @@ -226,5 +273,17 @@ void main() { lessThan(types.indexOf(OpacityModifier)), ); }); + + test('preserves a resolved non-default target order', () { + final tween = ModifierListTween( + begin: [OpacityModifier(0.2), PaddingModifier(const EdgeInsets.all(4))], + end: [OpacityModifier(0.8), PaddingModifier(const EdgeInsets.all(8))], + ); + + expect(tween.lerp(0.5)!.map((modifier) => modifier.runtimeType), [ + OpacityModifier, + PaddingModifier, + ]); + }); }); } diff --git a/packages/mix/test/src/modifiers/modifier_order_precedence_test.dart b/packages/mix/test/src/modifiers/modifier_order_precedence_test.dart index 37f5af6ff..483af855b 100644 --- a/packages/mix/test/src/modifiers/modifier_order_precedence_test.dart +++ b/packages/mix/test/src/modifiers/modifier_order_precedence_test.dart @@ -207,6 +207,82 @@ void main() { }, ); }); + + group('Modifier order phase animation lifecycle', () { + testWidgets('phase styles with modifiers mount below MixScope', ( + tester, + ) async { + final trigger = ValueNotifier(false); + addTearDown(trigger.dispose); + + final style = BoxStyler().phaseAnimation( + trigger: trigger, + phases: const [0, 1], + styleBuilder: (phase, style) => phase == 0 + ? style.wrap(WidgetModifierConfig.opacity(0.5)) + : style.wrap( + WidgetModifierConfig.padding(EdgeInsetsGeometryMix.all(8)), + ), + configBuilder: (_) => const CurveAnimationConfig( + duration: Duration(milliseconds: 100), + curve: Curves.linear, + ), + ); + + await tester.pumpWidget( + MaterialApp( + home: MixScope.empty(child: Box(style: style)), + ), + ); + + expect(tester.takeException(), isNull); + }); + + testWidgets('scope order changes re-resolve phase styles', (tester) async { + final trigger = ValueNotifier(false); + addTearDown(trigger.dispose); + + final style = BoxStyler().phaseAnimation( + trigger: trigger, + phases: const [0, 1], + styleBuilder: (_, style) => style.wrap(_opacityThenPadding()), + configBuilder: (_) => const CurveAnimationConfig( + duration: Duration(milliseconds: 100), + curve: Curves.linear, + ), + ); + final host = GlobalKey<_ScopeHostState>(); + final opacity = find.byWidgetPredicate( + (widget) => widget is Opacity && widget.opacity == 0.5, + ); + final padding = find.byWidgetPredicate( + (widget) => + widget is Padding && widget.padding == const EdgeInsets.all(10), + ); + + await tester.pumpWidget( + MaterialApp( + home: _ScopeHost( + key: host, + child: Box(style: style), + ), + ), + ); + trigger.value = true; + await tester.pump(); + await tester.pump(const Duration(milliseconds: 50)); + + expect(find.descendant(of: padding, matching: opacity), findsOneWidget); + + host.currentState!.update( + order: const [OpacityModifier, PaddingModifier], + ); + await tester.pump(); + + expect(find.descendant(of: opacity, matching: padding), findsOneWidget); + expect(find.descendant(of: padding, matching: opacity), findsNothing); + }); + }); } /// Test host that keeps its [child] instance stable across rebuilds so that From 0fd67bb2a2163fb4e5133b7d95a21e832e52b690 Mon Sep 17 00:00:00 2001 From: Leo Farias Date: Sat, 11 Jul 2026 13:41:33 -0400 Subject: [PATCH 3/3] fix(mix): preserve precedence for disjoint modifier tweens --- .../src/modifiers/widget_modifier_config.dart | 58 +++++++++++++-- .../modifiers/modifier_list_tween_test.dart | 71 ++++++++++++++++++- 2 files changed, 123 insertions(+), 6 deletions(-) diff --git a/packages/mix/lib/src/modifiers/widget_modifier_config.dart b/packages/mix/lib/src/modifiers/widget_modifier_config.dart index 5706f53f9..f2a1adf64 100644 --- a/packages/mix/lib/src/modifiers/widget_modifier_config.dart +++ b/packages/mix/lib/src/modifiers/widget_modifier_config.dart @@ -1,3 +1,5 @@ +import 'dart:collection'; + import 'package:flutter/widgets.dart'; import '../core/style.dart'; @@ -649,8 +651,8 @@ final class WidgetModifierConfig with Equatable { /// nearest scope order), then the package [_defaultOrder], then any remaining /// configured types in stable (first-seen) order. Each type is emitted once. /// -/// Shared by [WidgetModifierConfig.resolve] and -/// [WidgetModifierConfig.reorderModifiers]. +/// Shared by [WidgetModifierConfig.resolve], +/// [WidgetModifierConfig.reorderModifiers], and [ModifierListTween]. List _orderModifiers( List modifiers, { List preferredOrder = const [], @@ -673,7 +675,34 @@ List _orderModifiers( } } - return orderedSpecs; + return _ResolvedModifierList(orderedSpecs, preferredOrder); +} + +/// A resolved modifier list that retains the local/scope precedence used to +/// order it so transient animation unions can apply that same precedence. +final class _ResolvedModifierList extends ListBase { + final List preferredOrder; + final List _modifiers; + + _ResolvedModifierList( + List modifiers, + List preferredOrder, + ) : _modifiers = modifiers, + preferredOrder = List.unmodifiable(preferredOrder); + + @override + set length(int value) => _modifiers.length = value; + + @override + WidgetModifier operator [](int index) => _modifiers[index]; + + @override + void operator []=(int index, WidgetModifier value) { + _modifiers[index] = value; + } + + @override + int get length => _modifiers.length; } const _defaultOrder = [ @@ -791,7 +820,18 @@ final defaultModifier = { }; class ModifierListTween extends Tween?> { - ModifierListTween({super.begin, super.end}); + /// The target's resolved local/scope precedence, falling back to the + /// beginning precedence when there is no resolved target list. + final List? _preferredOrder; + + ModifierListTween({super.begin, super.end}) + : _preferredOrder = switch (end) { + _ResolvedModifierList() => end.preferredOrder, + _ => switch (begin) { + _ResolvedModifierList() => begin.preferredOrder, + _ => null, + }, + }; @override List? lerp(double t) { @@ -815,6 +855,7 @@ class ModifierListTween extends Tween?> { // Interpolate over the union of begin and target identities. Runtime type // is the matching identity (multiple instances of one type is out of scope). final lerped = []; + var emittedBeginningOnly = false; // Target members, in target order. for (final endModifier in endModifiers) { @@ -838,14 +879,21 @@ class ModifierListTween extends Tween?> { if (to != null) { // Beginning-only with a neutral default: interpolate begin -> neutral. lerped.add(beginModifier.lerp(to, t) as WidgetModifier); + emittedBeginningOnly = true; } else if (t < 0.5) { // Beginning-only without a neutral default: snap out at the midpoint. lerped.add(beginModifier); + emittedBeginningOnly = true; } } if (lerped.isEmpty) return null; - return lerped; + // A plain list with no beginning-only member already has the complete + // target order. Otherwise, order the transient union with the precedence + // retained during resolution, or the package default for direct callers. + if (_preferredOrder == null && !emittedBeginningOnly) return lerped; + + return _orderModifiers(lerped, preferredOrder: _preferredOrder ?? const []); } } diff --git a/packages/mix/test/src/modifiers/modifier_list_tween_test.dart b/packages/mix/test/src/modifiers/modifier_list_tween_test.dart index dc9022a46..d5087a2cd 100644 --- a/packages/mix/test/src/modifiers/modifier_list_tween_test.dart +++ b/packages/mix/test/src/modifiers/modifier_list_tween_test.dart @@ -2,6 +2,8 @@ import 'package:flutter/material.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:mix/mix.dart'; +import '../../helpers/testing_utils.dart'; + /// A custom [WidgetModifier] that is intentionally absent from `defaultModifier`, /// so it exercises the "no neutral default" interpolation path with a type that /// is not a built-in. @@ -38,6 +40,19 @@ X? _firstOfType>(List? list) { bool _has>(List? list) => _firstOfType(list) != null; +void _expectIntermediateOrder( + ModifierListTween tween, + List expectedTypes, +) { + for (final t in [0.25, 0.5, 0.75]) { + expect( + tween.lerp(t)!.map((modifier) => modifier.runtimeType), + expectedTypes, + reason: 'modifier precedence must remain stable at t=$t', + ); + } +} + void main() { // Membership snap point documented by ModifierListTween. const snap = 0.5; @@ -258,7 +273,61 @@ void main() { } }); - group('ModifierListTween.lerp — mixed membership keeps target order', () { + group('ModifierListTween.lerp — mixed membership preserves precedence', () { + test('uses package default order for disjoint endpoints', () { + final padding = [ + PaddingModifier(const EdgeInsets.all(8)), + ]; + final opacity = [OpacityModifier(0.5)]; + + _expectIntermediateOrder( + ModifierListTween(begin: padding, end: opacity), + [PaddingModifier, OpacityModifier], + ); + _expectIntermediateOrder( + ModifierListTween(begin: opacity, end: padding), + [PaddingModifier, OpacityModifier], + ); + }); + + test('uses style-local order for disjoint endpoints', () { + const localOrder = [OpacityModifier, PaddingModifier]; + final context = MockBuildContext(); + final opacity = WidgetModifierConfig.opacity( + 0.5, + ).orderOfModifiers(localOrder).resolve(context); + final padding = WidgetModifierConfig.padding( + EdgeInsetsGeometryMix.all(8), + ).orderOfModifiers(localOrder).resolve(context); + + _expectIntermediateOrder( + ModifierListTween(begin: padding, end: opacity), + localOrder, + ); + _expectIntermediateOrder( + ModifierListTween(begin: opacity, end: padding), + localOrder, + ); + }); + + test('uses MixScope order for disjoint endpoints', () { + const scopeOrder = [OpacityModifier, PaddingModifier]; + final context = MockBuildContext(orderOfModifiers: scopeOrder); + final opacity = WidgetModifierConfig.opacity(0.5).resolve(context); + final padding = WidgetModifierConfig.padding( + EdgeInsetsGeometryMix.all(8), + ).resolve(context); + + _expectIntermediateOrder( + ModifierListTween(begin: padding, end: opacity), + scopeOrder, + ); + _expectIntermediateOrder( + ModifierListTween(begin: opacity, end: padding), + scopeOrder, + ); + }); + test('union is ordered by the canonical modifier order', () { // Opacity is outermost-last in the default order; Padding precedes it. final tween = ModifierListTween(