Skip to content

Commit

Permalink
Reland 2: [CupertinoActionSheet] Match colors to native (flutter#150129)
Browse files Browse the repository at this point in the history 
Relands flutter#149568, which was reverted in flutter#149998 due to unverified golden tests post-commit from recent infra issues.

**New changes** (all contained in 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#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">
  • Loading branch information
@dkwingsmt
dkwingsmt authored Jun 12, 2024
1 parent 3832475 commit bb9daf5
Show file tree
Hide file tree
Showing 2 changed files with 153 additions and 22 deletions.
66 changes: 44 additions & 22 deletions packages/flutter/lib/src/cupertino/dialog.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand All @@ -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));
Expand Down Expand Up @@ -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),
),
),
),
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
);
Expand All @@ -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;
}
Expand Down
109 changes: 109 additions & 0 deletions packages/flutter/test/cupertino/action_sheet_test.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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,
Expand Down

0 comments on commit bb9daf5

Please sign in to comment.