From 96dae5e1e39cadff876ae1053795104fc38130de Mon Sep 17 00:00:00 2001 From: Taha Tesser Date: Fri, 12 May 2023 21:48:10 +0300 Subject: [PATCH] Fix focus behavior and provided thumb and focus color aren't applied to Cupertino variant of `Switch.adaptive` (#126688) fixes https://github.com/flutter/flutter/issues/126637 fixes https://github.com/flutter/flutter/issues/126669 ## ~~To be merged after https://github.com/flutter/flutter/pull/126684~~ ### Description 1. Fix the Cupertino variant of `Switch.adaptive` requires two tab key presses to focus. 2. Fix `thumbColor` & `focusColor` aren't applied. ### Before Requires two tab presses to focus. and `thumbColor` & `focusColor` aren't applied. https://github.com/flutter/flutter/assets/48603081/24635551-0794-443f-8320-32fdaa5de57a ### After Single tab key to focus (no additional focus node) and `thumbColor` & `focusColor` are applied. https://github.com/flutter/flutter/assets/48603081/9bf42fd8-c7e0-475a-b933-192a94650b0c --- packages/flutter/lib/src/material/switch.dart | 32 +++---- .../flutter/test/material/switch_test.dart | 94 ++++++++++++++++++- 2 files changed, 109 insertions(+), 17 deletions(-) diff --git a/packages/flutter/lib/src/material/switch.dart b/packages/flutter/lib/src/material/switch.dart index 2e0e914d033..ccc18c89d03 100644 --- a/packages/flutter/lib/src/material/switch.dart +++ b/packages/flutter/lib/src/material/switch.dart @@ -570,22 +570,22 @@ class Switch extends StatelessWidget { Widget _buildCupertinoSwitch(BuildContext context) { final Size size = _getSwitchSize(context); - return Focus( - focusNode: focusNode, - onFocusChange: onFocusChange, - autofocus: autofocus, - child: Container( - width: size.width, // Same size as the Material switch. - height: size.height, - alignment: Alignment.center, - child: CupertinoSwitch( - dragStartBehavior: dragStartBehavior, - value: value, - onChanged: onChanged, - activeColor: activeColor, - trackColor: inactiveTrackColor, - applyTheme: applyCupertinoTheme, - ), + return Container( + width: size.width, // Same size as the Material switch. + height: size.height, + alignment: Alignment.center, + child: CupertinoSwitch( + dragStartBehavior: dragStartBehavior, + value: value, + onChanged: onChanged, + activeColor: activeColor, + trackColor: inactiveTrackColor, + thumbColor: thumbColor?.resolve({}), + applyTheme: applyCupertinoTheme, + focusColor: focusColor, + focusNode: focusNode, + onFocusChange: onFocusChange, + autofocus: autofocus, ), ); } diff --git a/packages/flutter/test/material/switch_test.dart b/packages/flutter/test/material/switch_test.dart index 4ae75254fcd..41f18bb980a 100644 --- a/packages/flutter/test/material/switch_test.dart +++ b/packages/flutter/test/material/switch_test.dart @@ -791,7 +791,10 @@ void main() { testWidgets('Switch.adaptive', (WidgetTester tester) async { bool value = false; - const Color inactiveTrackColor = Colors.pink; + const Color activeTrackColor = Color(0xffff1200); + const Color inactiveTrackColor = Color(0xffff12ff); + const Color thumbColor = Color(0xffffff00); + const Color focusColor = Color(0xff00ff00); Widget buildFrame(TargetPlatform platform) { return MaterialApp( @@ -802,7 +805,10 @@ void main() { child: Center( child: Switch.adaptive( value: value, + activeColor: activeTrackColor, inactiveTrackColor: inactiveTrackColor, + thumbColor: const MaterialStatePropertyAll(thumbColor), + focusColor: focusColor, onChanged: (bool newValue) { setState(() { value = newValue; @@ -822,7 +828,10 @@ void main() { expect(find.byType(CupertinoSwitch), findsOneWidget, reason: 'on ${platform.name}'); final CupertinoSwitch adaptiveSwitch = tester.widget(find.byType(CupertinoSwitch)); + expect(adaptiveSwitch.activeColor, activeTrackColor, reason: 'on ${platform.name}'); expect(adaptiveSwitch.trackColor, inactiveTrackColor, reason: 'on ${platform.name}'); + expect(adaptiveSwitch.thumbColor, thumbColor, reason: 'on ${platform.name}'); + expect(adaptiveSwitch.focusColor, focusColor, reason: 'on ${platform.name}'); expect(value, isFalse, reason: 'on ${platform.name}'); await tester.tap(find.byType(Switch)); @@ -3278,6 +3287,89 @@ void main() { ); }); }); + + testWidgets('Switch.adaptive(Cupertino) is focusable and has correct focus color', (WidgetTester tester) async { + final FocusNode focusNode = FocusNode(debugLabel: 'Switch.adaptive'); + tester.binding.focusManager.highlightStrategy = FocusHighlightStrategy.alwaysTraditional; + bool value = true; + const Color focusColor = Color(0xffff0000); + + Widget buildApp({bool enabled = true}) { + return MaterialApp( + theme: ThemeData(platform: TargetPlatform.iOS), + home: Material( + child: StatefulBuilder( + builder: (BuildContext context, StateSetter setState) { + return Center( + child: Switch.adaptive( + value: value, + onChanged: enabled ? (bool newValue) { + setState(() { + value = newValue; + }); + } : null, + focusColor: focusColor, + focusNode: focusNode, + autofocus: true, + ), + ); + }, + ), + ), + ); + } + + await tester.pumpWidget(buildApp()); + await tester.pumpAndSettle(); + + expect(focusNode.hasPrimaryFocus, isTrue); + expect( + find.byType(CupertinoSwitch), + paints + ..rrect(color: const Color(0xff34c759)) + ..rrect(color: focusColor) + ..clipRRect() + ..rrect(color: const Color(0x26000000)) + ..rrect(color: const Color(0x0f000000)) + ..rrect(color: const Color(0x0a000000)) + ..rrect(color: const Color(0xffffffff)), + ); + + // Check the false value. + value = false; + await tester.pumpWidget(buildApp()); + await tester.pumpAndSettle(); + + expect(focusNode.hasPrimaryFocus, isTrue); + expect( + find.byType(CupertinoSwitch), + paints + ..rrect(color: const Color(0x28787880)) + ..rrect(color: focusColor) + ..clipRRect() + ..rrect(color: const Color(0x26000000)) + ..rrect(color: const Color(0x0f000000)) + ..rrect(color: const Color(0x0a000000)) + ..rrect(color: const Color(0xffffffff)), + ); + + // Check what happens when disabled. + value = false; + await tester.pumpWidget(buildApp(enabled: false)); + await tester.pumpAndSettle(); + + expect(focusNode.hasPrimaryFocus, isFalse); + expect( + find.byType(CupertinoSwitch), + paints + ..rrect(color: const Color(0x28787880)) + ..clipRRect() + ..rrect(color: const Color(0x26000000)) + ..rrect(color: const Color(0x0f000000)) + ..rrect(color: const Color(0x0a000000)) + ..rrect(color: const Color(0xffffffff)), + ); + }); } class DelayedImageProvider extends ImageProvider {