Unverified Commit f8ddd4b6 authored by Akito Nishiyama's avatar Akito Nishiyama Committed by GitHub

🎨: fix cupertionActionSheet design (#134345)

## Overview
Fixed the implementation of CupertionActionSheet based on [Apple Design Resource](https://www.figma.com/community/file/1248375255495415511).
I also deleted devicePixelRatio because Flutter uses logical pixels based on https://api.flutter.dev/flutter/dart-ui/FlutterView/devicePixelRatio.html.

ISSUE: #134539 

UPDATED GOLDEN: https://flutter-gold.skia.org/search?issue=134345&crs=github&patchsets=3&corpus=flutter

### Before
<img src="https://github.com/flutter/flutter/assets/40790076/8492fe5f-582f-4623-86eb-c60cb88d81a1" width=300>

### After
<img src="https://github.com/flutter/flutter/assets/40790076/fcdd7f7e-6ab5-4b68-a7b0-27a6fc2975b8" width=300>
parent 7bebdd3f
......@@ -54,7 +54,7 @@ const TextStyle _kCupertinoDialogActionStyle = TextStyle(
const TextStyle _kActionSheetActionStyle = TextStyle(
fontFamily: 'CupertinoSystemText',
inherit: false,
fontSize: 20.0,
fontSize: 17.0,
fontWeight: FontWeight.w400,
textBaseline: TextBaseline.alphabetic,
);
......@@ -71,7 +71,7 @@ const TextStyle _kActionSheetContentStyle = TextStyle(
// Generic constants shared between Dialog and ActionSheet.
const double _kBlurAmount = 20.0;
const double _kCornerRadius = 14.0;
const double _kDividerThickness = 1.0;
const double _kDividerThickness = 0.3;
// Dialog specific constants.
// iOS dialogs have a normal display width and another display width that is
......@@ -87,8 +87,8 @@ const double _kDialogMinButtonFontSize = 10.0;
const double _kActionSheetEdgeHorizontalPadding = 8.0;
const double _kActionSheetCancelButtonPadding = 8.0;
const double _kActionSheetEdgeVerticalPadding = 10.0;
const double _kActionSheetContentHorizontalPadding = 40.0;
const double _kActionSheetContentVerticalPadding = 14.0;
const double _kActionSheetContentHorizontalPadding = 16.0;
const double _kActionSheetContentVerticalPadding = 12.0;
const double _kActionSheetButtonHeight = 56.0;
// A translucent color that is painted on top of the blurred backdrop as the
......@@ -559,16 +559,16 @@ class _CupertinoActionSheetState extends State<CupertinoActionSheet> {
title: widget.title,
message: widget.message,
scrollController: _effectiveMessageScrollController,
titlePadding: const EdgeInsets.only(
titlePadding: EdgeInsets.only(
left: _kActionSheetContentHorizontalPadding,
right: _kActionSheetContentHorizontalPadding,
bottom: _kActionSheetContentVerticalPadding,
bottom: widget.message == null ? _kActionSheetContentVerticalPadding : 0.0,
top: _kActionSheetContentVerticalPadding,
),
messagePadding: EdgeInsets.only(
left: _kActionSheetContentHorizontalPadding,
right: _kActionSheetContentHorizontalPadding,
bottom: widget.title == null ? _kActionSheetContentVerticalPadding : 22.0,
bottom: _kActionSheetContentVerticalPadding,
top: widget.title == null ? _kActionSheetContentVerticalPadding : 0.0,
),
titleTextStyle: widget.message == null
......@@ -577,7 +577,7 @@ class _CupertinoActionSheetState extends State<CupertinoActionSheet> {
messageTextStyle: widget.title == null
? _kActionSheetContentStyle.copyWith(fontWeight: FontWeight.w600)
: _kActionSheetContentStyle,
additionalPaddingBetweenTitleAndMessage: const EdgeInsets.only(top: 8.0),
additionalPaddingBetweenTitleAndMessage: const EdgeInsets.only(top: 4.0),
);
content.add(Flexible(child: titleSection));
}
......@@ -819,7 +819,7 @@ class _CupertinoDialogRenderWidget extends RenderObjectWidget {
@override
RenderObject createRenderObject(BuildContext context) {
return _RenderCupertinoDialog(
dividerThickness: _kDividerThickness / MediaQuery.devicePixelRatioOf(context),
dividerThickness: _kDividerThickness,
isInAccessibilityMode: _isInAccessibilityMode(context) && !isActionSheet,
dividerColor: CupertinoDynamicColor.resolve(dividerColor, context),
isActionSheet: isActionSheet,
......@@ -1483,7 +1483,6 @@ class _CupertinoAlertActionSection extends StatelessWidget {
@override
Widget build(BuildContext context) {
final double devicePixelRatio = MediaQuery.devicePixelRatioOf(context);
final List<Widget> interactiveButtons = <Widget>[];
for (int i = 0; i < children.length; i += 1) {
......@@ -1500,7 +1499,7 @@ class _CupertinoAlertActionSection extends StatelessWidget {
controller: scrollController,
child: _CupertinoDialogActionsRenderWidget(
actionButtons: interactiveButtons,
dividerThickness: _kDividerThickness / devicePixelRatio,
dividerThickness: _kDividerThickness,
hasCancelButton: hasCancelButton,
isActionSheet: isActionSheet,
),
......
......@@ -347,11 +347,11 @@ void main() {
expect(tester.getCenter(find.widgetWithText(CupertinoActionSheetAction, 'Five')).dx, equals(400.0));
// Check that the action buttons are the correct heights.
expect(tester.getSize(find.widgetWithText(CupertinoActionSheetAction, 'One')).height, equals(92.0));
expect(tester.getSize(find.widgetWithText(CupertinoActionSheetAction, 'Two')).height, equals(92.0));
expect(tester.getSize(find.widgetWithText(CupertinoActionSheetAction, 'Three')).height, equals(92.0));
expect(tester.getSize(find.widgetWithText(CupertinoActionSheetAction, 'Four')).height, equals(92.0));
expect(tester.getSize(find.widgetWithText(CupertinoActionSheetAction, 'Five')).height, equals(92.0));
expect(tester.getSize(find.widgetWithText(CupertinoActionSheetAction, 'One')).height, equals(83.0));
expect(tester.getSize(find.widgetWithText(CupertinoActionSheetAction, 'Two')).height, equals(83.0));
expect(tester.getSize(find.widgetWithText(CupertinoActionSheetAction, 'Three')).height, equals(83.0));
expect(tester.getSize(find.widgetWithText(CupertinoActionSheetAction, 'Four')).height, equals(83.0));
expect(tester.getSize(find.widgetWithText(CupertinoActionSheetAction, 'Five')).height, equals(83.0));
});
testWidgetsWithLeakTracking('Content section is scrollable', (WidgetTester tester) async {
......@@ -517,7 +517,7 @@ void main() {
await tester.tap(find.text('Go'));
await tester.pump();
expect(tester.getSize(find.byType(CupertinoActionSheet)).height, moreOrLessEquals(132.33333333333334));
expect(tester.getSize(find.byType(CupertinoActionSheet)).height, moreOrLessEquals(132.3));
});
testWidgetsWithLeakTracking('1 action button with cancel button', (WidgetTester tester) async {
......@@ -574,7 +574,7 @@ void main() {
await tester.tap(find.text('Go'));
await tester.pump();
expect(findScrollableActionsSectionRenderBox(tester).size.height, moreOrLessEquals(112.33333333333331));
expect(findScrollableActionsSectionRenderBox(tester).size.height, moreOrLessEquals(112.3));
});
testWidgetsWithLeakTracking('3 action buttons with cancel button', (WidgetTester tester) async {
......@@ -608,7 +608,7 @@ void main() {
await tester.tap(find.text('Go'));
await tester.pump();
expect(findScrollableActionsSectionRenderBox(tester).size.height, moreOrLessEquals(168.66666666666669));
expect(findScrollableActionsSectionRenderBox(tester).size.height, moreOrLessEquals(168.6));
});
testWidgetsWithLeakTracking('4+ action buttons with cancel button', (WidgetTester tester) async {
......@@ -646,7 +646,7 @@ void main() {
await tester.tap(find.text('Go'));
await tester.pump();
expect(findScrollableActionsSectionRenderBox(tester).size.height, moreOrLessEquals(84.33333333333337));
expect(findScrollableActionsSectionRenderBox(tester).size.height, moreOrLessEquals(84.3));
});
testWidgetsWithLeakTracking('1 action button without cancel button', (WidgetTester tester) async {
......@@ -694,7 +694,7 @@ void main() {
await tester.tap(find.text('Go'));
await tester.pump();
expect(findScrollableActionsSectionRenderBox(tester).size.height, moreOrLessEquals(84.33333333333337));
expect(findScrollableActionsSectionRenderBox(tester).size.height, moreOrLessEquals(84.3));
});
testWidgetsWithLeakTracking('Action sheet with just cancel button is correct', (WidgetTester tester) async {
......@@ -784,7 +784,7 @@ void main() {
expect(tester.getBottomLeft(find.widgetWithText(CupertinoActionSheetAction, 'Cancel')).dy, 590.0);
expect(
tester.getBottomLeft(find.widgetWithText(CupertinoActionSheetAction, 'One')).dy,
moreOrLessEquals(469.66666666666663),
moreOrLessEquals(469.7),
);
expect(tester.getBottomLeft(find.widgetWithText(CupertinoActionSheetAction, 'Two')).dy, 526.0);
});
......@@ -820,46 +820,46 @@ void main() {
expect(tester.getTopLeft(find.byType(CupertinoActionSheet)).dy, 600.0);
await tester.pump(const Duration(milliseconds: 60));
expect(tester.getTopLeft(find.byType(CupertinoActionSheet)).dy, moreOrLessEquals(470.0, epsilon: 0.1));
expect(tester.getTopLeft(find.byType(CupertinoActionSheet)).dy, moreOrLessEquals(483.9, epsilon: 0.1));
await tester.pump(const Duration(milliseconds: 60));
expect(tester.getTopLeft(find.byType(CupertinoActionSheet)).dy, moreOrLessEquals(374.3, epsilon: 0.1));
expect(tester.getTopLeft(find.byType(CupertinoActionSheet)).dy, moreOrLessEquals(398.6, epsilon: 0.1));
await tester.pump(const Duration(milliseconds: 60));
expect(tester.getTopLeft(find.byType(CupertinoActionSheet)).dy, moreOrLessEquals(337.1, epsilon: 0.1));
expect(tester.getTopLeft(find.byType(CupertinoActionSheet)).dy, moreOrLessEquals(365.3, epsilon: 0.1));
await tester.pump(const Duration(milliseconds: 60));
expect(tester.getTopLeft(find.byType(CupertinoActionSheet)).dy, moreOrLessEquals(325.3, epsilon: 0.1));
expect(tester.getTopLeft(find.byType(CupertinoActionSheet)).dy, moreOrLessEquals(354.8, epsilon: 0.1));
await tester.pump(const Duration(milliseconds: 60));
expect(tester.getTopLeft(find.byType(CupertinoActionSheet)).dy, moreOrLessEquals(320.8, epsilon: 0.1));
expect(tester.getTopLeft(find.byType(CupertinoActionSheet)).dy, moreOrLessEquals(350.7, epsilon: 0.1));
await tester.pump(const Duration(milliseconds: 60));
expect(tester.getTopLeft(find.byType(CupertinoActionSheet)).dy, moreOrLessEquals(319.3, epsilon: 0.1));
expect(tester.getTopLeft(find.byType(CupertinoActionSheet)).dy, moreOrLessEquals(349.4, epsilon: 0.1));
// Action sheet has reached final height
await tester.pump(const Duration(milliseconds: 60));
expect(tester.getTopLeft(find.byType(CupertinoActionSheet)).dy, moreOrLessEquals(319.3, epsilon: 0.1));
expect(tester.getTopLeft(find.byType(CupertinoActionSheet)).dy, moreOrLessEquals(349.4, epsilon: 0.1));
// Exit animation
await tester.tapAt(const Offset(20.0, 20.0));
await tester.pump();
expect(tester.getTopLeft(find.byType(CupertinoActionSheet)).dy, moreOrLessEquals(319.3, epsilon: 0.1));
expect(tester.getTopLeft(find.byType(CupertinoActionSheet)).dy, moreOrLessEquals(349.4, epsilon: 0.1));
await tester.pump(const Duration(milliseconds: 60));
expect(tester.getTopLeft(find.byType(CupertinoActionSheet)).dy, moreOrLessEquals(449.3, epsilon: 0.1));
expect(tester.getTopLeft(find.byType(CupertinoActionSheet)).dy, moreOrLessEquals(465.5, epsilon: 0.1));
await tester.pump(const Duration(milliseconds: 60));
expect(tester.getTopLeft(find.byType(CupertinoActionSheet)).dy, moreOrLessEquals(544.9, epsilon: 0.1));
expect(tester.getTopLeft(find.byType(CupertinoActionSheet)).dy, moreOrLessEquals(550.8, epsilon: 0.1));
await tester.pump(const Duration(milliseconds: 60));
expect(tester.getTopLeft(find.byType(CupertinoActionSheet)).dy, moreOrLessEquals(582.1, epsilon: 0.1));
expect(tester.getTopLeft(find.byType(CupertinoActionSheet)).dy, moreOrLessEquals(584.1, epsilon: 0.1));
await tester.pump(const Duration(milliseconds: 60));
expect(tester.getTopLeft(find.byType(CupertinoActionSheet)).dy, moreOrLessEquals(593.9, epsilon: 0.1));
expect(tester.getTopLeft(find.byType(CupertinoActionSheet)).dy, moreOrLessEquals(594.6, epsilon: 0.1));
await tester.pump(const Duration(milliseconds: 60));
expect(tester.getTopLeft(find.byType(CupertinoActionSheet)).dy, moreOrLessEquals(598.5, epsilon: 0.1));
expect(tester.getTopLeft(find.byType(CupertinoActionSheet)).dy, moreOrLessEquals(598.7, epsilon: 0.1));
// Action sheet has disappeared
await tester.pump(const Duration(milliseconds: 60));
......@@ -897,23 +897,23 @@ void main() {
expect(tester.getTopLeft(find.byType(CupertinoActionSheet)).dy, 600.0);
await tester.pump(const Duration(milliseconds: 60));
expect(tester.getTopLeft(find.byType(CupertinoActionSheet)).dy, moreOrLessEquals(470.0, epsilon: 0.1));
expect(tester.getTopLeft(find.byType(CupertinoActionSheet)).dy, moreOrLessEquals(483.92863239836686, epsilon: 0.1));
await tester.pump(const Duration(milliseconds: 60));
expect(tester.getTopLeft(find.byType(CupertinoActionSheet)).dy, moreOrLessEquals(374.3, epsilon: 0.1));
expect(tester.getTopLeft(find.byType(CupertinoActionSheet)).dy, moreOrLessEquals(398.5571539306641, epsilon: 0.1));
await tester.pump(const Duration(milliseconds: 60));
expect(tester.getTopLeft(find.byType(CupertinoActionSheet)).dy, moreOrLessEquals(337.1, epsilon: 0.1));
expect(tester.getTopLeft(find.byType(CupertinoActionSheet)).dy, moreOrLessEquals(365.3034101784229, epsilon: 0.1));
// Exit animation
await tester.tapAt(const Offset(20.0, 20.0));
await tester.pump(const Duration(milliseconds: 60));
await tester.pump(const Duration(milliseconds: 60));
expect(tester.getTopLeft(find.byType(CupertinoActionSheet)).dy, moreOrLessEquals(374.3, epsilon: 0.1));
expect(tester.getTopLeft(find.byType(CupertinoActionSheet)).dy, moreOrLessEquals(398.5571539306641, epsilon: 0.1));
await tester.pump(const Duration(milliseconds: 60));
expect(tester.getTopLeft(find.byType(CupertinoActionSheet)).dy, moreOrLessEquals(470.0, epsilon: 0.1));
expect(tester.getTopLeft(find.byType(CupertinoActionSheet)).dy, moreOrLessEquals(483.92863239836686, epsilon: 0.1));
await tester.pump(const Duration(milliseconds: 60));
expect(tester.getTopLeft(find.byType(CupertinoActionSheet)).dy, 600.0);
......
......@@ -87,7 +87,7 @@ void main() {
});
testWidgetsWithLeakTracking('Dialog configurable to be barrier dismissible', (WidgetTester tester) async {
testWidgetsWithLeakTracking('Dialog configurable to be barrier dismissible', (WidgetTester tester) async {
await tester.pumpWidget(createAppWithCenteredButton(const Text('Go')));
final BuildContext context = tester.element(find.text('Go'));
......@@ -129,12 +129,12 @@ void main() {
testWidgetsWithLeakTracking('Dialog default action style', (WidgetTester tester) async {
await tester.pumpWidget(
CupertinoTheme(
data: const CupertinoThemeData(
primaryColor: CupertinoColors.systemGreen,
),
child: boilerplate(const CupertinoDialogAction(
child: Text('Ok'),
)),
data: const CupertinoThemeData(
primaryColor: CupertinoColors.systemGreen,
),
child: boilerplate(const CupertinoDialogAction(
child: Text('Ok'),
)),
),
);
......@@ -164,7 +164,7 @@ void main() {
),
);
final RichText cancelText = tester.widget<RichText>(
final RichText cancelText = tester.widget<RichText>(
find.descendant(of: find.text('Cancel'), matching: find.byType(RichText)),
);
......@@ -594,7 +594,7 @@ void main() {
await tester.pumpWidget(
createAppWithButtonThatLaunchesDialog(
dialogBuilder: (BuildContext context) {
dividerWidth = 1.0 / MediaQuery.devicePixelRatioOf(context);
dividerWidth = 0.3;
return CupertinoAlertDialog(
title: const Text('The Title'),
content: const Text('The message'),
......@@ -636,11 +636,10 @@ void main() {
testWidgetsWithLeakTracking('Actions section height for 2 stacked buttons with enough room is height of both buttons.', (WidgetTester tester) async {
final ScrollController scrollController = ScrollController();
addTearDown(scrollController.dispose);
late double dividerThickness; // Will be set when the dialog builder runs. Needs a BuildContext.
const double dividerThickness = 0.3;
await tester.pumpWidget(
createAppWithButtonThatLaunchesDialog(
dialogBuilder: (BuildContext context) {
dividerThickness = 1.0 / MediaQuery.devicePixelRatioOf(context);
return CupertinoAlertDialog(
title: const Text('The Title'),
content: const Text('The message'),
......@@ -707,7 +706,7 @@ void main() {
expect(
actionsSectionBox.size.height,
67.83333333333337,
67.80000000000001,
);
});
......@@ -789,8 +788,8 @@ void main() {
expect(option1ButtonBox.size.width, option2ButtonBox.size.width);
expect(option1ButtonBox.size.width, actionsSectionBox.size.width);
// Expected Height = button 1 + divider + 1/2 button 2 = 67.83333333333334
const double expectedHeight = 67.83333333333334;
// Expected Height = button 1 + divider + 1/2 button 2 = 67.80000000000001
const double expectedHeight = 67.80000000000001;
expect(
actionsSectionBox.size.height,
moreOrLessEquals(expectedHeight),
......@@ -849,11 +848,10 @@ void main() {
testWidgetsWithLeakTracking('Pressed button changes appearance and dividers disappear.', (WidgetTester tester) async {
final ScrollController scrollController = ScrollController();
addTearDown(scrollController.dispose);
late double dividerThickness; // Will be set when the dialog builder runs. Needs a BuildContext.
const double dividerThickness = 0.3;
await tester.pumpWidget(
createAppWithButtonThatLaunchesDialog(
dialogBuilder: (BuildContext context) {
dividerThickness = 1.0 / MediaQuery.devicePixelRatioOf(context);
return CupertinoAlertDialog(
title: const Text('The Title'),
content: const Text('The message'),
......@@ -893,10 +891,10 @@ void main() {
);
final Offset bottomDividerCenter = Offset(
secondButtonBox.size.width / 2.0,
firstButtonBox.size.height
+ dividerThickness
+ secondButtonBox.size.height
+ (0.5 * dividerThickness),
firstButtonBox.size.height +
dividerThickness +
secondButtonBox.size.height +
(0.5 * dividerThickness),
);
// Before pressing the button, verify following expectations:
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment