github/flutter
7
0
Fork 0
mirror of https://github.com/flutter/flutter.git synced 2025-06-03 00:51:18 +00:00
Code Issues Packages Projects Releases Wiki Activity

Fix the scrolling layout deviation of CupertinoActionSheet (#149439)

Browse Source 
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](https://github.com/flutter/flutter/pull/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.
This commit is contained in:
Tong Mu 2024-06-03 18:06:22 -07:00 committed by GitHub
parent c03fb95e9e
commit c246ecdf8e
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 11 additions and 33 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

40
packages/flutter/lib/src/cupertino/dialog.dart
View File

@ -631,7 +631,6 @@ class _CupertinoActionSheetState extends State<CupertinoActionSheet> {
child: _ActionSheetMainSheet(
scrollController: _effectiveActionScrollController,
hasContent: hasContent,
hasCancelButton: widget.cancelButton != null,
contentSection: Builder(builder: _buildContent),
actions: widget.actions,
dividerColor: _kActionSheetButtonDividerColor,
@ -918,7 +917,6 @@ class _ActionSheetMainSheet extends StatefulWidget {
required this.scrollController,
required this.actions,
required this.hasContent,
required this.hasCancelButton,
required this.contentSection,
required this.dividerColor,
});
@ -926,7 +924,6 @@ class _ActionSheetMainSheet extends StatefulWidget {
final ScrollController? scrollController;
final List<Widget>? actions;
final bool hasContent;
final bool hasCancelButton;
final Widget contentSection;
final Color dividerColor;
@ -980,25 +977,16 @@ class _ActionSheetMainSheetState extends State<_ActionSheetMainSheet> {
bool _hasActions() => (widget.actions?.length ?? 0) != 0;
// If `aggressivelyLayout` is true, then the content section takes as much
// space as needed up to `maxHeight`.
//
// If `aggressivelyLayout` is false, then the content section takes whatever
// space is left by the other sections.
Widget _buildContent({
required bool hasActions,
required bool aggressivelyLayout,
required double maxHeight,
}) {
if (hasActions && aggressivelyLayout) {
return ConstrainedBox(
constraints: BoxConstraints(
maxHeight: maxHeight,
),
Widget _buildContent({required bool hasActions, required double maxHeight}) {
if (!hasActions) {
return Flexible(
child: widget.contentSection,
);
}
return Flexible(
return ConstrainedBox(
constraints: BoxConstraints(
maxHeight: maxHeight,
),
child: widget.contentSection,
);
}
@ -1019,16 +1007,8 @@ class _ActionSheetMainSheetState extends State<_ActionSheetMainSheet> {
@override
Widget build(BuildContext context) {
// The layout rule:
//
// 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.
final int numActions = widget.actions?.length ?? 0;
final bool actionsMightScroll =
(numActions > 3 && widget.hasCancelButton) ||
(numActions > 1 && !widget.hasCancelButton) ;
// The content section takes priority for vertical space but must leave at
// least `_kActionSheetActionsSectionMinHeight` for the actions section.
final Color backgroundColor = CupertinoDynamicColor.resolve(_kActionSheetBackgroundColor, context);
return LayoutBuilder(
builder: (BuildContext context, BoxConstraints constraints) {
@ -1037,7 +1017,6 @@ class _ActionSheetMainSheetState extends State<_ActionSheetMainSheet> {
children: <Widget>[
_buildContent(
hasActions: _hasActions(),
aggressivelyLayout: actionsMightScroll,
maxHeight: constraints.maxHeight - _kActionSheetActionsSectionMinHeight - _kDividerThickness,
),
if (widget.hasContent && _hasActions())
@ -1046,7 +1025,6 @@ class _ActionSheetMainSheetState extends State<_ActionSheetMainSheet> {
hidden: false,
),
Flexible(
flex: actionsMightScroll ? 1 : 0,
child: Stack(
children: <Widget>[
Positioned.fill(

4
packages/flutter/test/cupertino/action_sheet_test.dart
View File

@ -719,7 +719,7 @@ void main() {
await tester.tap(find.text('Go'));
await tester.pump();
expect(findScrollableActionsSectionRenderBox(tester).size.height, moreOrLessEquals(112.3));
expect(findScrollableActionsSectionRenderBox(tester).size.height, moreOrLessEquals(84.3));
});
testWidgets('3 action buttons with cancel button', (WidgetTester tester) async {
@ -753,7 +753,7 @@ void main() {
await tester.tap(find.text('Go'));
await tester.pump();
expect(findScrollableActionsSectionRenderBox(tester).size.height, moreOrLessEquals(168.6));
expect(findScrollableActionsSectionRenderBox(tester).size.height, moreOrLessEquals(84.3));
});
testWidgets('4+ action buttons with cancel button', (WidgetTester tester) async {