Skip to content

Commit

Permalink
[CupertinoActionSheet] Fix the layout (part 1) (#149636)
Browse files Browse the repository at this point in the history
This PR fixes the general layout of `CupertinoActionSheet` to match the native behavior.

This PR adjusts the height of buttons, the height of the content section, the gap between the cancel button and the main sheet, and most importantly, the maximum height of the action sheet.

The maximum height is the trickiest part. I tried to figure out a rule, and found that the top padding only depends the type of the device - notch-less, notch, capsule - but there isn't a clear rule that can unify the 3 padding numbers. This PR uses linear interpolation as a heuristic algorithm. See the in-code comment for details. 
* What about iPad? Well, action sheets look completely different on iPad, more similar to a drop down menu. This might be fixed in the future.

### Tests

Among all the test changes, there are a few tests that have been converted to using `AnimationSheetRecorder` to verify the animation changes. Before the PR they were checking the height at each from, which is hard to reason whether a change makes sense, and hard to modify if anything needs changing.

### Result demo

The following images compares native(left) with Flutter after PR (right) by stacking them closely, and show that their layout really match almost pixel perfect.

<img width="455" alt="image" src="https://github.com/flutter/flutter/assets/1596656/f8be35bd-0da5-4908-92f7-7a1f4e999229">

_No notch (iPhone 13)_

<img width="405" alt="image" src="https://github.com/flutter/flutter/assets/1596656/54a37c2f-cd99-4e3b-86f0-045b1dfdbbb8">

_Notch (iPhone 13)_

<img width="385" alt="image" src="https://github.com/flutter/flutter/assets/1596656/546ab529-0b62-4e3d-9019-ef900d3552e5">

_Capsule (iPhone 15 Plus)_

<img width="1142" alt="image" src="https://github.com/flutter/flutter/assets/1596656/e06b6dac-dbcd-48f7-9dee-83700ae680e0">

_iPhone 13 landscape_

<img width="999" alt="image" src="https://github.com/flutter/flutter/assets/1596656/698cf530-51fc-4906-90a5-7a3ab626f489">

_All "capsule" devices share the same top padding in logical pixels (iPhone 15 Pro Max, iPhone 15 Pro, iPhone 15 Plus)_
  • Loading branch information
dkwingsmt authored Jun 22, 2024
1 parent 4a84fb0 commit 88e6f62
Show file tree
Hide file tree
Showing 2 changed files with 303 additions and 126 deletions.
100 changes: 91 additions & 9 deletions packages/flutter/lib/src/cupertino/dialog.dart
Original file line number Diff line number Diff line change
Expand Up @@ -86,13 +86,12 @@ const double _kDialogMinButtonHeight = 45.0;
const double _kDialogMinButtonFontSize = 10.0;

// ActionSheet specific constants.
const double _kActionSheetEdgeHorizontalPadding = 8.0;
const double _kActionSheetEdgePadding = 8.0;
const double _kActionSheetCancelButtonPadding = 8.0;
const double _kActionSheetEdgeVerticalPadding = 10.0;
const double _kActionSheetContentHorizontalPadding = 16.0;
const double _kActionSheetContentVerticalPadding = 12.0;
const double _kActionSheetButtonHeight = 56.0;
const double _kActionSheetActionsSectionMinHeight = 84.3;
const double _kActionSheetContentVerticalPadding = 13.5;
const double _kActionSheetButtonHeight = 57.0;
const double _kActionSheetActionsSectionMinHeight = 84.0;

// A translucent color that is painted on top of the blurred backdrop as the
// dialog's background color
Expand Down Expand Up @@ -915,10 +914,89 @@ class _CupertinoActionSheetState extends State<CupertinoActionSheet> {
);
}

// Given data point (x1, y1) and (x2, y2), derive the y corresponding to x
// using linear interpolation between the two data points, and extrapolates
// flatly beyond these points.
//
// (x2, y2)
// _____________
// /
// /
// _________/
// (x1, y1)
static double _lerp(double x, double x1, double y1, double x2, double y2) {
if (x <= x1) {
return y1;
} else if (x >= x2) {
return y2;
} else {
return Tween<double>(begin: y1, end: y2).transform(
(x - x1) / (x2 - x1)
);
}
}

// Derive the top padding, which is the distance between the top of a
// full-height action sheet and the top of the safe area.
//
// The algorithm and its values are derived from measuring on the simulator.
double _topPadding(BuildContext context) {
if (MediaQuery.orientationOf(context) == Orientation.landscape) {
return _kActionSheetEdgePadding;
}

// The top padding in portrait mode is in general close to the top view
// padding, but not always equal:
//
// | view padding | action sheet padding | ratio
// No notch (eg. iPhone SE) | 20.0 | 20.0 | 1.0
// Notch (eg. iPhone 13) | 47.0 | 47.0 | 1.0
// Capsule (eg. iPhone 15) | 59.0 | 54.0 | 0.915
//
// Currently, we cannot determine why the result changes on "capsules."
// Therefore, we'll hard code this rule, given the limited types of actual
// devices. To provide an algorithm that accepts arbitrary view padding, this
// function calculates the ratio as a continuous curve with linear
// interpolation.

// The x for lerp is the top view padding, while the y is ratio of
// action sheet padding versus top view padding.
const double viewPaddingData1 = 47.0;
const double paddingRatioData1 = 1.0;
const double viewPaddingData2 = 59.0;
const double paddingRatioData2 = 54.0 / 59.0;

final double currentViewPadding = MediaQuery.viewPaddingOf(context).top;

final double currentPaddingRatio = _lerp(
/* x= */currentViewPadding,
/* x1, y1= */viewPaddingData1, paddingRatioData1,
/* x2, y2= */viewPaddingData2, paddingRatioData2,
);
final double padding = (currentPaddingRatio * currentViewPadding).roundToDouble();
// In case there is no view padding, there should still be some space
// between the action sheet and the edge.
return math.max(padding, _kDialogEdgePadding);
}

@override
Widget build(BuildContext context) {
assert(debugCheckHasMediaQuery(context));

/*
* ╭─────────────────╮ ↑ ↑
* │ The title │ Content section |
* │ The message │ ↓ |
* ├─────────────────┤ ↑ Main sheet
* │ Action 1 │ | |
* ├─────────────────┤ Actions section |
* │ Action 2 │ | |
* ╰─────────────────╯ ↓ ↓
* ╭─────────────────╮
* │ Cancel │
* ╰─────────────────╯
*/

final List<Widget> children = <Widget>[
Flexible(
child: ClipRRect(
Expand All @@ -943,6 +1021,7 @@ class _CupertinoActionSheetState extends State<CupertinoActionSheet> {
};

return SafeArea(
minimum: const EdgeInsets.only(bottom: _kActionSheetEdgePadding),
child: ScrollConfiguration(
// A CupertinoScrollbar is built-in below
behavior: ScrollConfiguration.of(context).copyWith(scrollbars: false),
Expand All @@ -954,12 +1033,15 @@ class _CupertinoActionSheetState extends State<CupertinoActionSheet> {
child: CupertinoUserInterfaceLevel(
data: CupertinoUserInterfaceLevelData.elevated,
child: Padding(
padding: const EdgeInsets.symmetric(
horizontal: _kActionSheetEdgeHorizontalPadding,
vertical: _kActionSheetEdgeVerticalPadding,
padding: EdgeInsets.only(
left: _kActionSheetEdgePadding,
right: _kActionSheetEdgePadding,
top: _topPadding(context),
// The bottom padding is set on SafeArea.minimum, allowing it to
// be consumed by bottom view padding.
),
child: SizedBox(
width: actionSheetWidth - _kActionSheetEdgeHorizontalPadding * 2,
width: actionSheetWidth - _kActionSheetEdgePadding * 2,
child: _ActionSheetGestureDetector(
child: Semantics(
explicitChildNodes: true,
Expand Down
Loading

0 comments on commit 88e6f62

Please sign in to comment.