Skip to content

Commit c246ecd

Browse files
authored
Fix the scrolling layout deviation of CupertinoActionSheet (#149439)
This PR changes `CupertinoActionSheet`'s layout algorithm, so that the actions section always has the lower priority to take up vertical space with a fixed minimum height of 84.3 px. Currently `CupertinoActionSheet` uses a very complicated rule the determine the sizes of various sections: 1. If there are <= 3 buttons and a cancel button, or 1 button without a cancel button, then the actions section should never scroll. 2. Otherwise, then the content section takes priority to take over spaces but must leave at least `actionsMinHeight` for the actions section. This has been the case since `CupertinoActionSheet` was first introduced ([first PR](flutter/flutter#19232)). Maybe it was the case before, but it's no longer reproducible on the latest SwiftUI. The following images for comparison (left to right: Current Flutter, Flutter after this PR, SwiftUI). Pay attention to whether the actions section scrolls. (There are also some height/padding issues, which will be fixed in a future PR.) Two buttons: <img width="1006" alt="image" src="https://github.com/flutter/flutter/assets/1596656/27ca5df7-1140-4bfd-9a5f-3b5972632c92"> Three buttons: <img width="1025" alt="image" src="https://github.com/flutter/flutter/assets/1596656/f2be6a5c-1713-4ffe-9705-27a668e5133d"> Four buttons: <img width="1024" alt="image" src="https://github.com/flutter/flutter/assets/1596656/5b90d9f4-11ed-4c04-bdea-0b81b1d2bd3d"> In SwiftUI, the action section seems to always have a minimum height of ~84 pixels regardless of the number of buttons. Therefore this PR also fixed this behavior, and adjusted the related tests.
1 parent c03fb95 commit c246ecd

File tree

2 files changed

+11
-33
lines changed

2 files changed

+11
-33
lines changed

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

+9-31
Original file line numberDiff line numberDiff line change
@@ -631,7 +631,6 @@ class _CupertinoActionSheetState extends State<CupertinoActionSheet> {
631631
child: _ActionSheetMainSheet(
632632
scrollController: _effectiveActionScrollController,
633633
hasContent: hasContent,
634-
hasCancelButton: widget.cancelButton != null,
635634
contentSection: Builder(builder: _buildContent),
636635
actions: widget.actions,
637636
dividerColor: _kActionSheetButtonDividerColor,
@@ -918,15 +917,13 @@ class _ActionSheetMainSheet extends StatefulWidget {
918917
required this.scrollController,
919918
required this.actions,
920919
required this.hasContent,
921-
required this.hasCancelButton,
922920
required this.contentSection,
923921
required this.dividerColor,
924922
});
925923

926924
final ScrollController? scrollController;
927925
final List<Widget>? actions;
928926
final bool hasContent;
929-
final bool hasCancelButton;
930927
final Widget contentSection;
931928
final Color dividerColor;
932929

@@ -980,25 +977,16 @@ class _ActionSheetMainSheetState extends State<_ActionSheetMainSheet> {
980977

981978
bool _hasActions() => (widget.actions?.length ?? 0) != 0;
982979

983-
// If `aggressivelyLayout` is true, then the content section takes as much
984-
// space as needed up to `maxHeight`.
985-
//
986-
// If `aggressivelyLayout` is false, then the content section takes whatever
987-
// space is left by the other sections.
988-
Widget _buildContent({
989-
required bool hasActions,
990-
required bool aggressivelyLayout,
991-
required double maxHeight,
992-
}) {
993-
if (hasActions && aggressivelyLayout) {
994-
return ConstrainedBox(
995-
constraints: BoxConstraints(
996-
maxHeight: maxHeight,
997-
),
980+
Widget _buildContent({required bool hasActions, required double maxHeight}) {
981+
if (!hasActions) {
982+
return Flexible(
998983
child: widget.contentSection,
999984
);
1000985
}
1001-
return Flexible(
986+
return ConstrainedBox(
987+
constraints: BoxConstraints(
988+
maxHeight: maxHeight,
989+
),
1002990
child: widget.contentSection,
1003991
);
1004992
}
@@ -1019,16 +1007,8 @@ class _ActionSheetMainSheetState extends State<_ActionSheetMainSheet> {
10191007

10201008
@override
10211009
Widget build(BuildContext context) {
1022-
// The layout rule:
1023-
//
1024-
// 1. If there are <= 3 buttons and a cancel button, or 1 button without a
1025-
// cancel button, then the actions section should never scroll.
1026-
// 2. Otherwise, then the content section takes priority to take over spaces
1027-
// but must leave at least `actionsMinHeight` for the actions section.
1028-
final int numActions = widget.actions?.length ?? 0;
1029-
final bool actionsMightScroll =
1030-
(numActions > 3 && widget.hasCancelButton) ||
1031-
(numActions > 1 && !widget.hasCancelButton) ;
1010+
// The content section takes priority for vertical space but must leave at
1011+
// least `_kActionSheetActionsSectionMinHeight` for the actions section.
10321012
final Color backgroundColor = CupertinoDynamicColor.resolve(_kActionSheetBackgroundColor, context);
10331013
return LayoutBuilder(
10341014
builder: (BuildContext context, BoxConstraints constraints) {
@@ -1037,7 +1017,6 @@ class _ActionSheetMainSheetState extends State<_ActionSheetMainSheet> {
10371017
children: <Widget>[
10381018
_buildContent(
10391019
hasActions: _hasActions(),
1040-
aggressivelyLayout: actionsMightScroll,
10411020
maxHeight: constraints.maxHeight - _kActionSheetActionsSectionMinHeight - _kDividerThickness,
10421021
),
10431022
if (widget.hasContent && _hasActions())
@@ -1046,7 +1025,6 @@ class _ActionSheetMainSheetState extends State<_ActionSheetMainSheet> {
10461025
hidden: false,
10471026
),
10481027
Flexible(
1049-
flex: actionsMightScroll ? 1 : 0,
10501028
child: Stack(
10511029
children: <Widget>[
10521030
Positioned.fill(

packages/flutter/test/cupertino/action_sheet_test.dart

+2-2
Original file line numberDiff line numberDiff line change
@@ -719,7 +719,7 @@ void main() {
719719
await tester.tap(find.text('Go'));
720720
await tester.pump();
721721

722-
expect(findScrollableActionsSectionRenderBox(tester).size.height, moreOrLessEquals(112.3));
722+
expect(findScrollableActionsSectionRenderBox(tester).size.height, moreOrLessEquals(84.3));
723723
});
724724

725725
testWidgets('3 action buttons with cancel button', (WidgetTester tester) async {
@@ -753,7 +753,7 @@ void main() {
753753
await tester.tap(find.text('Go'));
754754
await tester.pump();
755755

756-
expect(findScrollableActionsSectionRenderBox(tester).size.height, moreOrLessEquals(168.6));
756+
expect(findScrollableActionsSectionRenderBox(tester).size.height, moreOrLessEquals(84.3));
757757
});
758758

759759
testWidgets('4+ action buttons with cancel button', (WidgetTester tester) async {

0 commit comments

Comments
 (0)