From 7b6d667701e38c1d9d700e2bcc29bc7a06598c59 Mon Sep 17 00:00:00 2001 From: yaakovschectman <109111084+yaakovschectman@users.noreply.github.com> Date: Wed, 12 Jun 2024 19:34:52 -0400 Subject: [PATCH] Revert "Reland 2: [CupertinoActionSheet] Match colors to native" (#150142) Reverts flutter/flutter#150129 Still introducing failure for flutter gold. The failure and error messages are specifically pointing to the test file modified by this commit. See https://logs.chromium.org/logs/flutter/buildbucket/cr-buildbucket/8745296364911252625/+/u/run_test.dart_for_framework_tests_shard_and_subshard_libraries/stdout --- .../flutter/lib/src/cupertino/dialog.dart | 66 ++++------- .../test/cupertino/action_sheet_test.dart | 109 ------------------ 2 files changed, 22 insertions(+), 153 deletions(-) diff --git a/packages/flutter/lib/src/cupertino/dialog.dart b/packages/flutter/lib/src/cupertino/dialog.dart index 15cd34a9e3c..c482b5fee49 100644 --- a/packages/flutter/lib/src/cupertino/dialog.dart +++ b/packages/flutter/lib/src/cupertino/dialog.dart @@ -65,9 +65,8 @@ 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. @@ -105,53 +104,35 @@ 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 _kDialogPressedColor = CupertinoDynamicColor.withBrightness( +const Color _kPressedColor = 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(0xFF494949), + darkColor: Color(0xFF49494B), ); // 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. -// Eyeballed from iOS 17 simulator. +// Extracted from https://developer.apple.com/design/resources/. const Color _kActionSheetBackgroundColor = CupertinoDynamicColor.withBrightness( - color: Color(0xC8FCFCFC), - darkColor: Color(0xBE292929), + color: Color(0xC7F9F9F9), + darkColor: Color(0xC7252525), ); // The gray color used for text that appears in the title area. -// Eyeballed from iOS 17 simulator. -const Color _kActionSheetContentTextColor = CupertinoDynamicColor.withBrightness( - color: Color(0x851D1D1D), - darkColor: Color(0x96F1F1F1), -); +// Extracted from https://developer.apple.com/design/resources/. +const Color _kActionSheetContentTextColor = Color(0xFF8F8F8F); // 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. -// Eyeballed from iOS 17 simulator. -const Color _kActionSheetButtonDividerColor = CupertinoDynamicColor.withBrightness( - color: Color(0xD4C9C9C9), - darkColor: Color(0xD57D7D7D), -); +// Eye-balled from iOS 13 beta simulator. +const Color _kActionSheetButtonDividerColor = _kActionSheetContentTextColor; // 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 @@ -860,9 +841,6 @@ 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, @@ -881,11 +859,11 @@ class _CupertinoActionSheetState extends State { top: widget.title == null ? _kActionSheetContentVerticalPadding : 0.0, ), titleTextStyle: widget.message == null - ? textStyle - : textStyle.copyWith(fontWeight: FontWeight.w600), + ? _kActionSheetContentStyle + : _kActionSheetContentStyle.copyWith(fontWeight: FontWeight.w600), messageTextStyle: widget.title == null - ? textStyle.copyWith(fontWeight: FontWeight.w600) - : textStyle, + ? _kActionSheetContentStyle.copyWith(fontWeight: FontWeight.w600) + : _kActionSheetContentStyle, additionalPaddingBetweenTitleAndMessage: const EdgeInsets.only(top: 4.0), ); content.add(Flexible(child: titleSection)); @@ -930,7 +908,7 @@ class _CupertinoActionSheetState extends State { hasContent: hasContent, contentSection: Builder(builder: _buildContent), actions: widget.actions, - dividerColor: CupertinoDynamicColor.resolve(_kActionSheetButtonDividerColor, context), + dividerColor: _kActionSheetButtonDividerColor, ), ), ), @@ -1137,19 +1115,19 @@ class _ActionSheetButtonBackgroundState extends State<_ActionSheetButtonBackgrou BorderRadius? borderRadius; if (!widget.isCancel) { backgroundColor = isBeingPressed - ? _kActionSheetPressedColor - : _kActionSheetBackgroundColor; + ? _kPressedColor + : CupertinoDynamicColor.resolve(_kActionSheetBackgroundColor, context); } else { backgroundColor = isBeingPressed - ? _kActionSheetCancelPressedColor - : _kActionSheetCancelColor; + ? _kActionSheetCancelPressedColor + : CupertinoColors.secondarySystemGroupedBackground; borderRadius = const BorderRadius.all(Radius.circular(_kCornerRadius)); } return MetaData( metaData: this, child: Container( decoration: BoxDecoration( - color: CupertinoDynamicColor.resolve(backgroundColor, context), + color: backgroundColor, borderRadius: borderRadius, ), child: widget.child, @@ -2291,7 +2269,7 @@ class _CupertinoDialogActionsRenderWidget extends MultiChildRenderObjectWidget { : _kCupertinoDialogWidth, dividerThickness: _dividerThickness, dialogColor: CupertinoDynamicColor.resolve(_kDialogColor, context), - dialogPressedColor: CupertinoDynamicColor.resolve(_kDialogPressedColor, context), + dialogPressedColor: CupertinoDynamicColor.resolve(_kPressedColor, context), dividerColor: CupertinoDynamicColor.resolve(CupertinoColors.separator, context), hasCancelButton: _hasCancelButton, ); @@ -2305,7 +2283,7 @@ class _CupertinoDialogActionsRenderWidget extends MultiChildRenderObjectWidget { : _kCupertinoDialogWidth ..dividerThickness = _dividerThickness ..dialogColor = CupertinoDynamicColor.resolve(_kDialogColor, context) - ..dialogPressedColor = CupertinoDynamicColor.resolve(_kDialogPressedColor, context) + ..dialogPressedColor = CupertinoDynamicColor.resolve(_kPressedColor, 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 ebed820a5f9..84aa8e32397 100644 --- a/packages/flutter/test/cupertino/action_sheet_test.dart +++ b/packages/flutter/test/cupertino/action_sheet_test.dart @@ -18,71 +18,6 @@ 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( @@ -1740,50 +1675,6 @@ 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,