From bb9daf5572902207a3f2c92f1f29ffd756a265c8 Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Wed, 12 Jun 2024 15:12:27 -0700 Subject: [PATCH] Reland 2: [CupertinoActionSheet] Match colors to native (#150129) Relands #149568, which was reverted in https://github.com/flutter/flutter/pull/149998 due to unverified golden tests post-commit from recent infra issues. **New changes** (all contained in https://github.com/flutter/flutter/pull/150129/commits/5ca5139d4351e9d2f2e7563de23781ac8c123adb ): * Fixes a problem within the tests "Overall looks correctly under x theme" where the button press is not applied to the golden file. * Dark theme now uses colors different from the light theme. It was an issue reported in https://github.com/flutter/flutter/pull/149568#issuecomment-2161392381. I made the mistake because the XCode preview doesn't correctly apply dark theme, which made me think the dark theme uses the same colors. As shown in the following table, the dark theme colors also achieved deviation of <=1. image Screenshot comparison: (left to right: native, Flutter after PR, Flutter before PR) image --- .../flutter/lib/src/cupertino/dialog.dart | 66 +++++++---- .../test/cupertino/action_sheet_test.dart | 109 ++++++++++++++++++ 2 files changed, 153 insertions(+), 22 deletions(-) diff --git a/packages/flutter/lib/src/cupertino/dialog.dart b/packages/flutter/lib/src/cupertino/dialog.dart index c482b5fee49..15cd34a9e3c 100644 --- a/packages/flutter/lib/src/cupertino/dialog.dart +++ b/packages/flutter/lib/src/cupertino/dialog.dart @@ -65,8 +65,9 @@ const TextStyle _kActionSheetContentStyle = TextStyle( inherit: false, fontSize: 13.0, fontWeight: FontWeight.w400, - color: _kActionSheetContentTextColor, textBaseline: TextBaseline.alphabetic, + // The `color` is configured by _kActionSheetContentTextColor to be dynamic on + // context. ); // Generic constants shared between Dialog and ActionSheet. @@ -104,35 +105,53 @@ const Color _kDialogColor = CupertinoDynamicColor.withBrightness( // Translucent light gray that is painted on top of the blurred backdrop as the // background color of a pressed button. // Eyeballed from iOS 13 beta simulator. -const Color _kPressedColor = CupertinoDynamicColor.withBrightness( +const Color _kDialogPressedColor = CupertinoDynamicColor.withBrightness( color: Color(0xFFE1E1E1), darkColor: Color(0xFF2E2E2E), ); +// Translucent light gray that is painted on top of the blurred backdrop as the +// background color of a pressed button. +// Eyeballed from iOS 17 simulator. +const Color _kActionSheetPressedColor = CupertinoDynamicColor.withBrightness( + color: Color(0xCAE0E0E0), + darkColor: Color(0xC1515151), +); + +const Color _kActionSheetCancelColor = CupertinoDynamicColor.withBrightness( + color: Color(0xFFFFFFFF), + darkColor: Color(0xFF2C2C2C), +); const Color _kActionSheetCancelPressedColor = CupertinoDynamicColor.withBrightness( color: Color(0xFFECECEC), - darkColor: Color(0xFF49494B), + darkColor: Color(0xFF494949), ); // Translucent, very light gray that is painted on top of the blurred backdrop // as the action sheet's background color. // TODO(LongCatIsLooong): https://github.com/flutter/flutter/issues/39272. Use // System Materials once we have them. -// Extracted from https://developer.apple.com/design/resources/. +// Eyeballed from iOS 17 simulator. const Color _kActionSheetBackgroundColor = CupertinoDynamicColor.withBrightness( - color: Color(0xC7F9F9F9), - darkColor: Color(0xC7252525), + color: Color(0xC8FCFCFC), + darkColor: Color(0xBE292929), ); // The gray color used for text that appears in the title area. -// Extracted from https://developer.apple.com/design/resources/. -const Color _kActionSheetContentTextColor = Color(0xFF8F8F8F); +// Eyeballed from iOS 17 simulator. +const Color _kActionSheetContentTextColor = CupertinoDynamicColor.withBrightness( + color: Color(0x851D1D1D), + darkColor: Color(0x96F1F1F1), +); // Translucent gray that is painted on top of the blurred backdrop in the gap // areas between the content section and actions section, as well as between // buttons. -// Eye-balled from iOS 13 beta simulator. -const Color _kActionSheetButtonDividerColor = _kActionSheetContentTextColor; +// Eyeballed from iOS 17 simulator. +const Color _kActionSheetButtonDividerColor = CupertinoDynamicColor.withBrightness( + color: Color(0xD4C9C9C9), + darkColor: Color(0xD57D7D7D), +); // The alert dialog layout policy changes depending on whether the user is using // a "regular" font size vs a "large" font size. This is a spectrum. There are @@ -841,6 +860,9 @@ class _CupertinoActionSheetState extends State { Widget _buildContent(BuildContext context) { final List content = []; + final TextStyle textStyle = _kActionSheetContentStyle.copyWith( + color: CupertinoDynamicColor.resolve(_kActionSheetContentTextColor, context), + ); if (hasContent) { final Widget titleSection = _CupertinoAlertContentSection( title: widget.title, @@ -859,11 +881,11 @@ class _CupertinoActionSheetState extends State { top: widget.title == null ? _kActionSheetContentVerticalPadding : 0.0, ), titleTextStyle: widget.message == null - ? _kActionSheetContentStyle - : _kActionSheetContentStyle.copyWith(fontWeight: FontWeight.w600), + ? textStyle + : textStyle.copyWith(fontWeight: FontWeight.w600), messageTextStyle: widget.title == null - ? _kActionSheetContentStyle.copyWith(fontWeight: FontWeight.w600) - : _kActionSheetContentStyle, + ? textStyle.copyWith(fontWeight: FontWeight.w600) + : textStyle, additionalPaddingBetweenTitleAndMessage: const EdgeInsets.only(top: 4.0), ); content.add(Flexible(child: titleSection)); @@ -908,7 +930,7 @@ class _CupertinoActionSheetState extends State { hasContent: hasContent, contentSection: Builder(builder: _buildContent), actions: widget.actions, - dividerColor: _kActionSheetButtonDividerColor, + dividerColor: CupertinoDynamicColor.resolve(_kActionSheetButtonDividerColor, context), ), ), ), @@ -1115,19 +1137,19 @@ class _ActionSheetButtonBackgroundState extends State<_ActionSheetButtonBackgrou BorderRadius? borderRadius; if (!widget.isCancel) { backgroundColor = isBeingPressed - ? _kPressedColor - : CupertinoDynamicColor.resolve(_kActionSheetBackgroundColor, context); + ? _kActionSheetPressedColor + : _kActionSheetBackgroundColor; } else { backgroundColor = isBeingPressed - ? _kActionSheetCancelPressedColor - : CupertinoColors.secondarySystemGroupedBackground; + ? _kActionSheetCancelPressedColor + : _kActionSheetCancelColor; borderRadius = const BorderRadius.all(Radius.circular(_kCornerRadius)); } return MetaData( metaData: this, child: Container( decoration: BoxDecoration( - color: backgroundColor, + color: CupertinoDynamicColor.resolve(backgroundColor, context), borderRadius: borderRadius, ), child: widget.child, @@ -2269,7 +2291,7 @@ class _CupertinoDialogActionsRenderWidget extends MultiChildRenderObjectWidget { : _kCupertinoDialogWidth, dividerThickness: _dividerThickness, dialogColor: CupertinoDynamicColor.resolve(_kDialogColor, context), - dialogPressedColor: CupertinoDynamicColor.resolve(_kPressedColor, context), + dialogPressedColor: CupertinoDynamicColor.resolve(_kDialogPressedColor, context), dividerColor: CupertinoDynamicColor.resolve(CupertinoColors.separator, context), hasCancelButton: _hasCancelButton, ); @@ -2283,7 +2305,7 @@ class _CupertinoDialogActionsRenderWidget extends MultiChildRenderObjectWidget { : _kCupertinoDialogWidth ..dividerThickness = _dividerThickness ..dialogColor = CupertinoDynamicColor.resolve(_kDialogColor, context) - ..dialogPressedColor = CupertinoDynamicColor.resolve(_kPressedColor, context) + ..dialogPressedColor = CupertinoDynamicColor.resolve(_kDialogPressedColor, context) ..dividerColor = CupertinoDynamicColor.resolve(CupertinoColors.separator, context) ..hasCancelButton = _hasCancelButton; } diff --git a/packages/flutter/test/cupertino/action_sheet_test.dart b/packages/flutter/test/cupertino/action_sheet_test.dart index 84aa8e32397..ebed820a5f9 100644 --- a/packages/flutter/test/cupertino/action_sheet_test.dart +++ b/packages/flutter/test/cupertino/action_sheet_test.dart @@ -18,6 +18,71 @@ import 'package:flutter_test/flutter_test.dart'; import '../widgets/semantics_tester.dart'; void main() { + testWidgets('Overall looks correctly under light theme', (WidgetTester tester) async { + await tester.pumpWidget( + TestScaffoldApp( + theme: const CupertinoThemeData(brightness: Brightness.light), + actionSheet: CupertinoActionSheet( + message: const Text('The title'), + actions: [ + CupertinoActionSheetAction(child: const Text('One'), onPressed: () {}), + CupertinoActionSheetAction(child: const Text('Two'), onPressed: () {}), + ], + cancelButton: CupertinoActionSheetAction(child: const Text('Cancel'), onPressed: () {}), + ), + ), + ); + + await tester.tap(find.text('Go')); + await tester.pumpAndSettle(); + + final TestGesture gesture = await tester.startGesture(tester.getCenter(find.text('One'))); + await tester.pumpAndSettle(); + // This golden file also verifies the structure of an action sheet that + // has a message, no title, and no overscroll for any sections (in contrast + // to cupertinoActionSheet.dark-theme.png). + await expectLater( + find.byType(CupertinoApp), + matchesGoldenFile('cupertinoActionSheet.overall-light-theme.png'), + ); + + await gesture.up(); + }); + + testWidgets('Overall looks correctly under dark theme', (WidgetTester tester) async { + await tester.pumpWidget( + TestScaffoldApp( + theme: const CupertinoThemeData(brightness: Brightness.dark), + actionSheet: CupertinoActionSheet( + title: const Text('The title'), + message: const Text('The message'), + actions: List.generate(20, (int i) => + CupertinoActionSheetAction( + onPressed: () {}, + child: Text('Button $i'), + ), + ), + cancelButton: CupertinoActionSheetAction(child: const Text('Cancel'), onPressed: () {}), + ), + ), + ); + + await tester.tap(find.text('Go')); + await tester.pumpAndSettle(); + + final TestGesture gesture = await tester.startGesture(tester.getCenter(find.text('Button 0'))); + await tester.pumpAndSettle(); + // This golden file also verifies the structure of an action sheet that + // has both a message and a title, and an overscrolled action section (in + // contrast to cupertinoActionSheet.light-theme.png). + await expectLater( + find.byType(CupertinoApp), + matchesGoldenFile('cupertinoActionSheet.overall-dark-theme.png'), + ); + + await gesture.up(); + }); + testWidgets('Verify that a tap on modal barrier dismisses an action sheet', (WidgetTester tester) async { await tester.pumpWidget( createAppWithButtonThatLaunchesActionSheet( @@ -1675,6 +1740,50 @@ Widget createAppWithButtonThatLaunchesActionSheet(Widget actionSheet) { ); } +// Shows an app that has a button with text "Go", and clicking this button +// displays the `actionSheet` and hides the button. +// +// The `theme` will be applied to the app and determines the background. +class TestScaffoldApp extends StatefulWidget { + const TestScaffoldApp({super.key, required this.theme, required this.actionSheet}); + final CupertinoThemeData theme; + final Widget actionSheet; + + @override + TestScaffoldAppState createState() => TestScaffoldAppState(); +} + +class TestScaffoldAppState extends State { + bool _pressedButton = false; + + @override + Widget build(BuildContext context) { + return CupertinoApp( + theme: widget.theme, + home: Builder(builder: (BuildContext context) => + CupertinoPageScaffold( + child: Center( + child: _pressedButton ? Container() : CupertinoButton( + onPressed: () { + setState(() { + _pressedButton = true; + }); + showCupertinoModalPopup( + context: context, + builder: (BuildContext context) { + return widget.actionSheet; + }, + ); + }, + child: const Text('Go'), + ), + ), + ), + ), + ); + } +} + Widget boilerplate(Widget child) { return Directionality( textDirection: TextDirection.ltr,