Skip to content

Commit bb9daf5

Browse files
authored
Reland 2: [CupertinoActionSheet] Match colors to native (#150129)
Relands #149568, which was reverted in flutter/flutter#149998 due to unverified golden tests post-commit from recent infra issues. **New changes** (all contained in flutter/flutter@5ca5139 ): * 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 flutter/flutter#149568 (comment). 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">
1 parent 3832475 commit bb9daf5

2 files changed

Lines changed: 153 additions & 22 deletions

File tree

packages/flutter/lib/src/cupertino/dialog.dart

Lines changed: 44 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -65,8 +65,9 @@ const TextStyle _kActionSheetContentStyle = TextStyle(
6565
inherit: false,
6666
fontSize: 13.0,
6767
fontWeight: FontWeight.w400,
68-
color: _kActionSheetContentTextColor,
6968
textBaseline: TextBaseline.alphabetic,
69+
// The `color` is configured by _kActionSheetContentTextColor to be dynamic on
70+
// context.
7071
);
7172

7273
// Generic constants shared between Dialog and ActionSheet.
@@ -104,35 +105,53 @@ const Color _kDialogColor = CupertinoDynamicColor.withBrightness(
104105
// Translucent light gray that is painted on top of the blurred backdrop as the
105106
// background color of a pressed button.
106107
// Eyeballed from iOS 13 beta simulator.
107-
const Color _kPressedColor = CupertinoDynamicColor.withBrightness(
108+
const Color _kDialogPressedColor = CupertinoDynamicColor.withBrightness(
108109
color: Color(0xFFE1E1E1),
109110
darkColor: Color(0xFF2E2E2E),
110111
);
111112

113+
// Translucent light gray that is painted on top of the blurred backdrop as the
114+
// background color of a pressed button.
115+
// Eyeballed from iOS 17 simulator.
116+
const Color _kActionSheetPressedColor = CupertinoDynamicColor.withBrightness(
117+
color: Color(0xCAE0E0E0),
118+
darkColor: Color(0xC1515151),
119+
);
120+
121+
const Color _kActionSheetCancelColor = CupertinoDynamicColor.withBrightness(
122+
color: Color(0xFFFFFFFF),
123+
darkColor: Color(0xFF2C2C2C),
124+
);
112125
const Color _kActionSheetCancelPressedColor = CupertinoDynamicColor.withBrightness(
113126
color: Color(0xFFECECEC),
114-
darkColor: Color(0xFF49494B),
127+
darkColor: Color(0xFF494949),
115128
);
116129

117130
// Translucent, very light gray that is painted on top of the blurred backdrop
118131
// as the action sheet's background color.
119132
// TODO(LongCatIsLooong): https://github.com/flutter/flutter/issues/39272. Use
120133
// System Materials once we have them.
121-
// Extracted from https://developer.apple.com/design/resources/.
134+
// Eyeballed from iOS 17 simulator.
122135
const Color _kActionSheetBackgroundColor = CupertinoDynamicColor.withBrightness(
123-
color: Color(0xC7F9F9F9),
124-
darkColor: Color(0xC7252525),
136+
color: Color(0xC8FCFCFC),
137+
darkColor: Color(0xBE292929),
125138
);
126139

127140
// The gray color used for text that appears in the title area.
128-
// Extracted from https://developer.apple.com/design/resources/.
129-
const Color _kActionSheetContentTextColor = Color(0xFF8F8F8F);
141+
// Eyeballed from iOS 17 simulator.
142+
const Color _kActionSheetContentTextColor = CupertinoDynamicColor.withBrightness(
143+
color: Color(0x851D1D1D),
144+
darkColor: Color(0x96F1F1F1),
145+
);
130146

131147
// Translucent gray that is painted on top of the blurred backdrop in the gap
132148
// areas between the content section and actions section, as well as between
133149
// buttons.
134-
// Eye-balled from iOS 13 beta simulator.
135-
const Color _kActionSheetButtonDividerColor = _kActionSheetContentTextColor;
150+
// Eyeballed from iOS 17 simulator.
151+
const Color _kActionSheetButtonDividerColor = CupertinoDynamicColor.withBrightness(
152+
color: Color(0xD4C9C9C9),
153+
darkColor: Color(0xD57D7D7D),
154+
);
136155

137156
// The alert dialog layout policy changes depending on whether the user is using
138157
// a "regular" font size vs a "large" font size. This is a spectrum. There are
@@ -841,6 +860,9 @@ class _CupertinoActionSheetState extends State<CupertinoActionSheet> {
841860

842861
Widget _buildContent(BuildContext context) {
843862
final List<Widget> content = <Widget>[];
863+
final TextStyle textStyle = _kActionSheetContentStyle.copyWith(
864+
color: CupertinoDynamicColor.resolve(_kActionSheetContentTextColor, context),
865+
);
844866
if (hasContent) {
845867
final Widget titleSection = _CupertinoAlertContentSection(
846868
title: widget.title,
@@ -859,11 +881,11 @@ class _CupertinoActionSheetState extends State<CupertinoActionSheet> {
859881
top: widget.title == null ? _kActionSheetContentVerticalPadding : 0.0,
860882
),
861883
titleTextStyle: widget.message == null
862-
? _kActionSheetContentStyle
863-
: _kActionSheetContentStyle.copyWith(fontWeight: FontWeight.w600),
884+
? textStyle
885+
: textStyle.copyWith(fontWeight: FontWeight.w600),
864886
messageTextStyle: widget.title == null
865-
? _kActionSheetContentStyle.copyWith(fontWeight: FontWeight.w600)
866-
: _kActionSheetContentStyle,
887+
? textStyle.copyWith(fontWeight: FontWeight.w600)
888+
: textStyle,
867889
additionalPaddingBetweenTitleAndMessage: const EdgeInsets.only(top: 4.0),
868890
);
869891
content.add(Flexible(child: titleSection));
@@ -908,7 +930,7 @@ class _CupertinoActionSheetState extends State<CupertinoActionSheet> {
908930
hasContent: hasContent,
909931
contentSection: Builder(builder: _buildContent),
910932
actions: widget.actions,
911-
dividerColor: _kActionSheetButtonDividerColor,
933+
dividerColor: CupertinoDynamicColor.resolve(_kActionSheetButtonDividerColor, context),
912934
),
913935
),
914936
),
@@ -1115,19 +1137,19 @@ class _ActionSheetButtonBackgroundState extends State<_ActionSheetButtonBackgrou
11151137
BorderRadius? borderRadius;
11161138
if (!widget.isCancel) {
11171139
backgroundColor = isBeingPressed
1118-
? _kPressedColor
1119-
: CupertinoDynamicColor.resolve(_kActionSheetBackgroundColor, context);
1140+
? _kActionSheetPressedColor
1141+
: _kActionSheetBackgroundColor;
11201142
} else {
11211143
backgroundColor = isBeingPressed
1122-
? _kActionSheetCancelPressedColor
1123-
: CupertinoColors.secondarySystemGroupedBackground;
1144+
? _kActionSheetCancelPressedColor
1145+
: _kActionSheetCancelColor;
11241146
borderRadius = const BorderRadius.all(Radius.circular(_kCornerRadius));
11251147
}
11261148
return MetaData(
11271149
metaData: this,
11281150
child: Container(
11291151
decoration: BoxDecoration(
1130-
color: backgroundColor,
1152+
color: CupertinoDynamicColor.resolve(backgroundColor, context),
11311153
borderRadius: borderRadius,
11321154
),
11331155
child: widget.child,
@@ -2269,7 +2291,7 @@ class _CupertinoDialogActionsRenderWidget extends MultiChildRenderObjectWidget {
22692291
: _kCupertinoDialogWidth,
22702292
dividerThickness: _dividerThickness,
22712293
dialogColor: CupertinoDynamicColor.resolve(_kDialogColor, context),
2272-
dialogPressedColor: CupertinoDynamicColor.resolve(_kPressedColor, context),
2294+
dialogPressedColor: CupertinoDynamicColor.resolve(_kDialogPressedColor, context),
22732295
dividerColor: CupertinoDynamicColor.resolve(CupertinoColors.separator, context),
22742296
hasCancelButton: _hasCancelButton,
22752297
);
@@ -2283,7 +2305,7 @@ class _CupertinoDialogActionsRenderWidget extends MultiChildRenderObjectWidget {
22832305
: _kCupertinoDialogWidth
22842306
..dividerThickness = _dividerThickness
22852307
..dialogColor = CupertinoDynamicColor.resolve(_kDialogColor, context)
2286-
..dialogPressedColor = CupertinoDynamicColor.resolve(_kPressedColor, context)
2308+
..dialogPressedColor = CupertinoDynamicColor.resolve(_kDialogPressedColor, context)
22872309
..dividerColor = CupertinoDynamicColor.resolve(CupertinoColors.separator, context)
22882310
..hasCancelButton = _hasCancelButton;
22892311
}

packages/flutter/test/cupertino/action_sheet_test.dart

Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,71 @@ import 'package:flutter_test/flutter_test.dart';
1818
import '../widgets/semantics_tester.dart';
1919

2020
void main() {
21+
testWidgets('Overall looks correctly under light theme', (WidgetTester tester) async {
22+
await tester.pumpWidget(
23+
TestScaffoldApp(
24+
theme: const CupertinoThemeData(brightness: Brightness.light),
25+
actionSheet: CupertinoActionSheet(
26+
message: const Text('The title'),
27+
actions: <Widget>[
28+
CupertinoActionSheetAction(child: const Text('One'), onPressed: () {}),
29+
CupertinoActionSheetAction(child: const Text('Two'), onPressed: () {}),
30+
],
31+
cancelButton: CupertinoActionSheetAction(child: const Text('Cancel'), onPressed: () {}),
32+
),
33+
),
34+
);
35+
36+
await tester.tap(find.text('Go'));
37+
await tester.pumpAndSettle();
38+
39+
final TestGesture gesture = await tester.startGesture(tester.getCenter(find.text('One')));
40+
await tester.pumpAndSettle();
41+
// This golden file also verifies the structure of an action sheet that
42+
// has a message, no title, and no overscroll for any sections (in contrast
43+
// to cupertinoActionSheet.dark-theme.png).
44+
await expectLater(
45+
find.byType(CupertinoApp),
46+
matchesGoldenFile('cupertinoActionSheet.overall-light-theme.png'),
47+
);
48+
49+
await gesture.up();
50+
});
51+
52+
testWidgets('Overall looks correctly under dark theme', (WidgetTester tester) async {
53+
await tester.pumpWidget(
54+
TestScaffoldApp(
55+
theme: const CupertinoThemeData(brightness: Brightness.dark),
56+
actionSheet: CupertinoActionSheet(
57+
title: const Text('The title'),
58+
message: const Text('The message'),
59+
actions: List<Widget>.generate(20, (int i) =>
60+
CupertinoActionSheetAction(
61+
onPressed: () {},
62+
child: Text('Button $i'),
63+
),
64+
),
65+
cancelButton: CupertinoActionSheetAction(child: const Text('Cancel'), onPressed: () {}),
66+
),
67+
),
68+
);
69+
70+
await tester.tap(find.text('Go'));
71+
await tester.pumpAndSettle();
72+
73+
final TestGesture gesture = await tester.startGesture(tester.getCenter(find.text('Button 0')));
74+
await tester.pumpAndSettle();
75+
// This golden file also verifies the structure of an action sheet that
76+
// has both a message and a title, and an overscrolled action section (in
77+
// contrast to cupertinoActionSheet.light-theme.png).
78+
await expectLater(
79+
find.byType(CupertinoApp),
80+
matchesGoldenFile('cupertinoActionSheet.overall-dark-theme.png'),
81+
);
82+
83+
await gesture.up();
84+
});
85+
2186
testWidgets('Verify that a tap on modal barrier dismisses an action sheet', (WidgetTester tester) async {
2287
await tester.pumpWidget(
2388
createAppWithButtonThatLaunchesActionSheet(
@@ -1675,6 +1740,50 @@ Widget createAppWithButtonThatLaunchesActionSheet(Widget actionSheet) {
16751740
);
16761741
}
16771742

1743+
// Shows an app that has a button with text "Go", and clicking this button
1744+
// displays the `actionSheet` and hides the button.
1745+
//
1746+
// The `theme` will be applied to the app and determines the background.
1747+
class TestScaffoldApp extends StatefulWidget {
1748+
const TestScaffoldApp({super.key, required this.theme, required this.actionSheet});
1749+
final CupertinoThemeData theme;
1750+
final Widget actionSheet;
1751+
1752+
@override
1753+
TestScaffoldAppState createState() => TestScaffoldAppState();
1754+
}
1755+
1756+
class TestScaffoldAppState extends State<TestScaffoldApp> {
1757+
bool _pressedButton = false;
1758+
1759+
@override
1760+
Widget build(BuildContext context) {
1761+
return CupertinoApp(
1762+
theme: widget.theme,
1763+
home: Builder(builder: (BuildContext context) =>
1764+
CupertinoPageScaffold(
1765+
child: Center(
1766+
child: _pressedButton ? Container() : CupertinoButton(
1767+
onPressed: () {
1768+
setState(() {
1769+
_pressedButton = true;
1770+
});
1771+
showCupertinoModalPopup<void>(
1772+
context: context,
1773+
builder: (BuildContext context) {
1774+
return widget.actionSheet;
1775+
},
1776+
);
1777+
},
1778+
child: const Text('Go'),
1779+
),
1780+
),
1781+
),
1782+
),
1783+
);
1784+
}
1785+
}
1786+
16781787
Widget boilerplate(Widget child) {
16791788
return Directionality(
16801789
textDirection: TextDirection.ltr,

0 commit comments

Comments
 (0)