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 5ca5139d43 ):
* 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.

<img width="1091" alt="image" src="https://github.com/flutter/flutter/assets/1596656/f4acda2b-1857-449c-8c1b-1f48afeb9095">

Screenshot comparison: (left to right: native, Flutter after PR, Flutter before PR)
<img width="1286" alt="image" src="https://github.com/flutter/flutter/assets/1596656/580eef1f-a7f9-45d9-a7c8-fab0ca9606e3">
This commit is contained in:
Tong Mu 2024-06-12 15:12:27 -07:00 committed by GitHub
parent 3832475930
commit bb9daf5572
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 153 additions and 22 deletions

View File

@ -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<CupertinoActionSheet> {
Widget _buildContent(BuildContext context) {
final List<Widget> content = <Widget>[];
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<CupertinoActionSheet> {
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<CupertinoActionSheet> {
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;
}

View File

@ -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: <Widget>[
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<Widget>.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<TestScaffoldApp> {
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<void>(
context: context,
builder: (BuildContext context) {
return widget.actionSheet;
},
);
},
child: const Text('Go'),
),
),
),
),
);
}
}
Widget boilerplate(Widget child) {
return Directionality(
textDirection: TextDirection.ltr,