diff --git a/packages/mix/lib/src/core/styled_widget.dart b/packages/mix/lib/src/core/styled_widget.dart index 29f156ca5..bd56a2e31 100644 --- a/packages/mix/lib/src/core/styled_widget.dart +++ b/packages/mix/lib/src/core/styled_widget.dart @@ -140,7 +140,14 @@ class SpecBuilder extends StatelessWidget { _hasWidgetStateVariant && MixWidgetState.of(context) == null; if (needsWidgetState || controller != null) { - current = Interactable(controller: controller, child: current); + final canRequestFocus = + style.variants.values.any((attr) => attr.variant is OnFocusedVariant); + + current = Interactable( + controller: controller, + canRequestFocus: canRequestFocus, + child: current, + ); } // Otherwise, directly build the mixed child widget diff --git a/packages/mix/lib/src/core/widget_state/internal/interactive_mix_state.dart b/packages/mix/lib/src/core/widget_state/internal/interactive_mix_state.dart index f32fa0cc1..09f7fac81 100644 --- a/packages/mix/lib/src/core/widget_state/internal/interactive_mix_state.dart +++ b/packages/mix/lib/src/core/widget_state/internal/interactive_mix_state.dart @@ -88,18 +88,21 @@ class _InteractiveStateBuilderState extends State { @override Widget build(BuildContext context) { - return FocusableActionDetector( - enabled: widget.enabled, - focusNode: widget.focusNode, - autofocus: widget.autofocus, - shortcuts: widget.shortcuts, - actions: widget.actions, - onShowFocusHighlight: _onShowFocusHighlight, - onShowHoverHighlight: _onShowHoverHighlight, - onFocusChange: widget.onFocusChange, - mouseCursor: widget.mouseCursor, - includeFocusSemantics: false, - child: widget.child, + return ExcludeFocus( + excluding: !widget.canRequestFocus, + child: FocusableActionDetector( + enabled: widget.enabled, + focusNode: widget.focusNode, + autofocus: widget.autofocus, + shortcuts: widget.shortcuts, + actions: widget.actions, + onShowFocusHighlight: _onShowFocusHighlight, + onShowHoverHighlight: _onShowHoverHighlight, + onFocusChange: widget.onFocusChange, + mouseCursor: widget.mouseCursor, + includeFocusSemantics: false, + child: widget.child, + ), ); } } diff --git a/packages/mix/test/src/core/styled_widget_test.dart b/packages/mix/test/src/core/styled_widget_test.dart index 37ef371a7..07e38db50 100644 --- a/packages/mix/test/src/core/styled_widget_test.dart +++ b/packages/mix/test/src/core/styled_widget_test.dart @@ -85,27 +85,70 @@ void main() { ); }, ); - group('SpecBuilder', () { - testWidgets( - 'When a SpecBuilder has a controller, it should wrap the child with Interactable', - (tester) async { - final controller = MixWidgetStateController(); - await tester.pumpWidget( - SpecBuilder( - controller: controller, - builder: (context) => const SizedBox(), + + testWidgets( + 'When a SpecBuilder has a controller, it should wrap the child with Interactable', + (tester) async { + final controller = MixWidgetStateController(); + await tester.pumpWidget( + SpecBuilder( + controller: controller, + builder: (context) => const SizedBox(), + ), + ); + + expect(find.byType(InteractiveMixStateWidget), findsOneWidget); + }, + ); + + testWidgets( + 'When a SpecBuilder has a style with MixWidgetStateVariant, it should wrap the child with Interactable', + (tester) async { + await tester.pumpWidget( + SpecBuilder( + style: Style( + $box.color( + Colors.red, + ), + $on.press( + $box.color(Colors.blue), + ), ), - ); + builder: (context) => const SizedBox(), + ), + ); - expect(find.byType(InteractiveMixStateWidget), findsOneWidget); - }, - ); + expect(find.byType(InteractiveMixStateWidget), findsOneWidget); + }, + ); - testWidgets( - 'When a SpecBuilder has a style with MixWidgetStateVariant, it should wrap the child with Interactable', - (tester) async { - await tester.pumpWidget( - SpecBuilder( + testWidgets( + 'When a SpecBuilder has a style with OnHoverVariant, it should wrap the child with MouseRegionMixStateWidget', + (tester) async { + await tester.pumpWidget( + SpecBuilder( + style: Style( + $box.color(Colors.red), + $on.hover.event((event) { + return const Style.empty(); + }), + ), + builder: (context) => const SizedBox(), + ), + ); + + expect(find.byType(MouseRegionMixStateWidget), findsOneWidget); + expect(find.byType(InteractiveMixStateWidget), findsOneWidget); + }, + ); + + // Create a test that already has a Interactable and ther is only one InteractiveMixStateWidget + testWidgets( + 'When a SpecBuilder has a controller and a style with MixWidgetStateVariant, it should wrap the child with Interactable', + (tester) async { + await tester.pumpWidget( + Interactable( + child: SpecBuilder( style: Style( $box.color( Colors.red, @@ -116,113 +159,136 @@ void main() { ), builder: (context) => const SizedBox(), ), - ); + ), + ); - expect(find.byType(InteractiveMixStateWidget), findsOneWidget); - }, - ); + expect(find.byType(InteractiveMixStateWidget), findsOneWidget); + }, + ); - testWidgets( - 'When a SpecBuilder has a style with OnHoverVariant, it should wrap the child with MouseRegionMixStateWidget', - (tester) async { - await tester.pumpWidget( - SpecBuilder( - style: Style( - $box.color(Colors.red), - $on.hover.event((event) { - return const Style.empty(); - }), - ), - builder: (context) => const SizedBox(), - ), - ); + // do not add interactive mix state if its not needed + testWidgets( + 'When a SpecBuilder has a controller and a style with MixWidgetStateVariant, it should wrap the child with Interactable', + (tester) async { + await tester.pumpWidget( + SpecBuilder( + style: Style($box.color(Colors.red)), + builder: (context) => const SizedBox(), + ), + ); - expect(find.byType(MouseRegionMixStateWidget), findsOneWidget); - expect(find.byType(InteractiveMixStateWidget), findsOneWidget); - }, - ); + expect(find.byType(InteractiveMixStateWidget), findsNothing); + }, + ); - // Create a test that already has a Interactable and ther is only one InteractiveMixStateWidget - testWidgets( - 'When a SpecBuilder has a controller and a style with MixWidgetStateVariant, it should wrap the child with Interactable', - (tester) async { - await tester.pumpWidget( - Interactable( - child: SpecBuilder( - style: Style( - $box.color( - Colors.red, - ), - $on.press( - $box.color(Colors.blue), - ), - ), - builder: (context) => const SizedBox(), - ), - ), - ); + testWidgets( + 'When a SpecBuilder has a controller it should wrap the widget', + (tester) async { + await tester.pumpWidget( + SpecBuilder( + controller: MixWidgetStateController(), + style: Style($box.color(Colors.red)), + builder: (context) => const SizedBox(), + ), + ); - expect(find.byType(InteractiveMixStateWidget), findsOneWidget); - }, - ); + expect(find.byType(InteractiveMixStateWidget), findsOneWidget); + }, + ); - // do not add interactive mix state if its not needed - testWidgets( - 'When a SpecBuilder has a controller and a style with MixWidgetStateVariant, it should wrap the child with Interactable', - (tester) async { - await tester.pumpWidget( - SpecBuilder( - style: Style($box.color(Colors.red)), - builder: (context) => const SizedBox(), - ), - ); + testWidgets( + 'When a SpecBuilder has an animated style, it should use RenderAnimatedModifiers', + (tester) async { + await tester.pumpWidget( + SpecBuilder( + style: Style($box.color(Colors.red)).animate(), + builder: (context) => const SizedBox(), + ), + ); - expect(find.byType(InteractiveMixStateWidget), findsNothing); - }, - ); + expect(find.byType(RenderAnimatedModifiers), findsOneWidget); + }, + ); - testWidgets( - 'When a SpecBuilder has a controller it should wrap the widget', - (tester) async { - await tester.pumpWidget( - SpecBuilder( - controller: MixWidgetStateController(), - style: Style($box.color(Colors.red)), - builder: (context) => const SizedBox(), - ), - ); + testWidgets( + 'When a SpecBuilder has a non-animated style, it should use RenderModifiers', + (tester) async { + await tester.pumpWidget( + SpecBuilder( + style: Style($box.color(Colors.red)), + builder: (context) => const SizedBox(), + ), + ); - expect(find.byType(InteractiveMixStateWidget), findsOneWidget); - }, - ); + expect(find.byType(RenderModifiers), findsOneWidget); + }, + ); - testWidgets( - 'When a SpecBuilder has an animated style, it should use RenderAnimatedModifiers', - (tester) async { - await tester.pumpWidget( - SpecBuilder( - style: Style($box.color(Colors.red)).animate(), - builder: (context) => const SizedBox(), - ), - ); + testWidgets( + 'When a SpecBuilder has a non-animated style, it should use RenderModifiers', + (tester) async { + await tester.pumpWidget( + SpecBuilder( + style: Style($box.color(Colors.red)), + builder: (context) => const SizedBox(), + ), + ); - expect(find.byType(RenderAnimatedModifiers), findsOneWidget); - }, - ); + expect(find.byType(RenderModifiers), findsOneWidget); + }, + ); - testWidgets( - 'When a SpecBuilder has a non-animated style, it should use RenderModifiers', + testWidgets( + 'should set canRequestFocus to false if there is no \$on.focus in Style', (tester) async { - await tester.pumpWidget( - SpecBuilder( - style: Style($box.color(Colors.red)), - builder: (context) => const SizedBox(), - ), - ); - - expect(find.byType(RenderModifiers), findsOneWidget); - }, + await _testFocusability( + tester, + style: Style( + $box.color(Colors.blue), + $on.press( + $box.color(Colors.blue), + ), + ), + expectedHasFocus: false, ); }); + + testWidgets( + 'should set canRequestFocus to true if there is a \$on.focus in Style', + (tester) async { + await _testFocusability( + tester, + style: Style( + $box.color(Colors.red), + $on.focus( + $box.color(Colors.blue), + ), + ), + expectedHasFocus: true, + ); + }, + ); }); } + +Future _testFocusability( + WidgetTester tester, { + required Style style, + required bool expectedHasFocus, +}) async { + await tester.pumpWidget( + SpecBuilder( + style: style, + builder: (context) => const SizedBox(), + ), + ); + + expect(find.byType(InteractiveMixStateWidget), findsOneWidget); + expect( + tester + .firstWidget( + find.byType(InteractiveMixStateWidget)) + .canRequestFocus, + expectedHasFocus, + ); +} diff --git a/packages/mix/test/src/widgets/pressable/pressable_widget_test.dart b/packages/mix/test/src/widgets/pressable/pressable_widget_test.dart index 7fd5fb8d0..82aaca1f3 100644 --- a/packages/mix/test/src/widgets/pressable/pressable_widget_test.dart +++ b/packages/mix/test/src/widgets/pressable/pressable_widget_test.dart @@ -63,6 +63,32 @@ void main() { reason: 'Third Pressable should not have disabled state'); }); + testWidgets('should not be focusable when canRequestFocus is false', + (tester) async { + await _testFocusability( + tester, + builder: (focusNode, child) => Pressable( + canRequestFocus: false, + focusNode: focusNode, + child: child, + ), + expectedHasFocus: false, + ); + }); + + testWidgets('should be focusable when canRequestFocus is true', + (tester) async { + await _testFocusability( + tester, + builder: (focusNode, child) => Pressable( + canRequestFocus: true, + focusNode: focusNode, + child: child, + ), + expectedHasFocus: true, + ); + }); + testWidgets('is disposing the controller correcly', (widgetTester) async { await widgetTester.pumpWidget(const _DisposalPressable()); }); @@ -639,6 +665,32 @@ void main() { expect(onTapCalled, isTrue); }); + + testWidgets('should be focusable when canRequestFocus is true', + (tester) async { + await _testFocusability( + tester, + builder: (focusNode, child) => Interactable( + canRequestFocus: true, + focusNode: focusNode, + child: child, + ), + expectedHasFocus: true, + ); + }); + + testWidgets('should not be focusable when canRequestFocus is false', + (tester) async { + await _testFocusability( + tester, + builder: (focusNode, child) => Interactable( + canRequestFocus: false, + focusNode: focusNode, + child: child, + ), + expectedHasFocus: false, + ); + }); }); } @@ -732,3 +784,23 @@ class _DisposalPressableState extends State<_DisposalPressable> { ); } } + +Future _testFocusability( + WidgetTester tester, { + required bool expectedHasFocus, + required Widget Function(FocusNode focusNode, Widget child) builder, +}) async { + final focusNode = FocusNode(); + await tester.pumpWidget( + builder( + focusNode, + Container(), + ), + ); + + focusNode.requestFocus(); + await tester.pump(); + + expect(find.byType(FocusableActionDetector), findsOneWidget); + expect(focusNode.hasFocus, expectedHasFocus); +}