Skip to content

Conversation

@dkwingsmt
Copy link
Contributor

@dkwingsmt dkwingsmt commented May 31, 2024

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). 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:
image

Three buttons:
image

Four buttons:
image

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.

Pre-launch Checklist

If you need help, consider asking for advice on the #hackers-new channel on Discord.

@github-actions github-actions bot added framework flutter/packages/flutter repository. See also f: labels. f: cupertino flutter/packages/flutter/cupertino repository labels May 31, 2024
@dkwingsmt dkwingsmt force-pushed the as-fix-scroll branch 2 times, most recently from 123f495 to 4a00b77 Compare May 31, 2024 21:07
@dkwingsmt dkwingsmt marked this pull request as ready for review May 31, 2024 23:49
@dkwingsmt dkwingsmt requested a review from chunhtai May 31, 2024 23:49
Copy link
Contributor

@chunhtai chunhtai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@dkwingsmt dkwingsmt added the autosubmit Merge PR when tree becomes green via auto submit App label Jun 3, 2024
@auto-submit auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Jun 3, 2024
@auto-submit
Copy link
Contributor

auto-submit bot commented Jun 3, 2024

auto label is removed for flutter/flutter/149439, due to - The status or check suite Windows build_tests_1_7 has failed. Please fix the issues identified (or deflake) before re-applying this label.

@dkwingsmt dkwingsmt added the autosubmit Merge PR when tree becomes green via auto submit App label Jun 4, 2024
@auto-submit auto-submit bot merged commit c246ecd into flutter:master Jun 4, 2024
hello-coder-xu pushed a commit to hello-coder-xu/flutter that referenced this pull request Jun 4, 2024
…pdate

* master: (168 commits)
  Fix the scrolling layout deviation of `CupertinoActionSheet` (flutter#149439)
  Roll Flutter Engine from 60a7bb2353b6 to a6aa5d826649 (2 revisions) (flutter#149627)
  Roll Flutter Engine from ea72558be758 to 60a7bb2353b6 (2 revisions) (flutter#149623)
  Place `flutter_gpu` in the package cache. (flutter#149299)
  Switch to triage-* labels for platform package triage (flutter#149614)
  Roll pub packages (flutter#149617)
  Fixes multi line textfield hint text gets ellipsized (flutter#148423)
  Support failures-only and silent reporters in `flutter test` (flutter#148739)
  [CupertinoActionSheet] Fix overflow of the overscroll section when the user scrolls far (flutter#149542)
  Fix InputDecorator.prefixIcon color when disabled (flutter#149595)
  Added filter callback on dropdown menu (flutter#143939)
  update generated localized message files in the stocks test app (flutter#148741)
  Add a simplified SimpleCascadingMenuApp example (flutter#149147)
  Reland "Prevent LayoutBuilder from rebuilding more than once (flutter#147856)" (flutter#149303)
  Move some benchmarks from MotoG4 to Mokey (flutter#149567)
  Roll Packages from d8e8e8c to 11e192a (2 revisions) (flutter#149596)
  Cleanup triage reports from docs/ (flutter#149545)
  Roll Flutter Engine from d81edf635a9f to ea72558be758 (1 revision) (flutter#149590)
  Roll Flutter Engine from b0f4d7459708 to d81edf635a9f (1 revision) (flutter#149468)
  Roll Flutter Engine from 40b868efcc46 to b0f4d7459708 (1 revision) (flutter#149467)
  ...
hello-coder-xu added a commit to hello-coder-xu/flutter that referenced this pull request Jun 6, 2024
…pdate

* master: (168 commits)
  Fix the scrolling layout deviation of `CupertinoActionSheet` (flutter#149439)
  Roll Flutter Engine from 60a7bb2353b6 to a6aa5d826649 (2 revisions) (flutter#149627)
  Roll Flutter Engine from ea72558be758 to 60a7bb2353b6 (2 revisions) (flutter#149623)
  Place `flutter_gpu` in the package cache. (flutter#149299)
  Switch to triage-* labels for platform package triage (flutter#149614)
  Roll pub packages (flutter#149617)
  Fixes multi line textfield hint text gets ellipsized (flutter#148423)
  Support failures-only and silent reporters in `flutter test` (flutter#148739)
  [CupertinoActionSheet] Fix overflow of the overscroll section when the user scrolls far (flutter#149542)
  Fix InputDecorator.prefixIcon color when disabled (flutter#149595)
  Added filter callback on dropdown menu (flutter#143939)
  update generated localized message files in the stocks test app (flutter#148741)
  Add a simplified SimpleCascadingMenuApp example (flutter#149147)
  Reland "Prevent LayoutBuilder from rebuilding more than once (flutter#147856)" (flutter#149303)
  Move some benchmarks from MotoG4 to Mokey (flutter#149567)
  Roll Packages from d8e8e8c to 11e192a (2 revisions) (flutter#149596)
  Cleanup triage reports from docs/ (flutter#149545)
  Roll Flutter Engine from d81edf635a9f to ea72558be758 (1 revision) (flutter#149590)
  Roll Flutter Engine from b0f4d7459708 to d81edf635a9f (1 revision) (flutter#149468)
  Roll Flutter Engine from 40b868efcc46 to b0f4d7459708 (1 revision) (flutter#149467)
  ...
hello-coder-xu added a commit to hello-coder-xu/flutter that referenced this pull request Jun 9, 2024
…pdate

* master: (181 commits)
  Fix the scrolling layout deviation of `CupertinoActionSheet` (flutter#149439)
  Roll Flutter Engine from 60a7bb2353b6 to a6aa5d826649 (2 revisions) (flutter#149627)
  Roll Flutter Engine from ea72558be758 to 60a7bb2353b6 (2 revisions) (flutter#149623)
  Place `flutter_gpu` in the package cache. (flutter#149299)
  Switch to triage-* labels for platform package triage (flutter#149614)
  Roll pub packages (flutter#149617)
  Fixes multi line textfield hint text gets ellipsized (flutter#148423)
  Support failures-only and silent reporters in `flutter test` (flutter#148739)
  [CupertinoActionSheet] Fix overflow of the overscroll section when the user scrolls far (flutter#149542)
  Fix InputDecorator.prefixIcon color when disabled (flutter#149595)
  Added filter callback on dropdown menu (flutter#143939)
  update generated localized message files in the stocks test app (flutter#148741)
  Add a simplified SimpleCascadingMenuApp example (flutter#149147)
  Reland "Prevent LayoutBuilder from rebuilding more than once (flutter#147856)" (flutter#149303)
  Move some benchmarks from MotoG4 to Mokey (flutter#149567)
  Roll Packages from d8e8e8c to 11e192a (2 revisions) (flutter#149596)
  Cleanup triage reports from docs/ (flutter#149545)
  Roll Flutter Engine from d81edf635a9f to ea72558be758 (1 revision) (flutter#149590)
  Roll Flutter Engine from b0f4d7459708 to d81edf635a9f (1 revision) (flutter#149468)
  Roll Flutter Engine from 40b868efcc46 to b0f4d7459708 (1 revision) (flutter#149467)
  ...
hello-coder-xu added a commit to hello-coder-xu/flutter that referenced this pull request Jun 11, 2024
* master: (181 commits)
  Fix the scrolling layout deviation of `CupertinoActionSheet` (flutter#149439)
  Roll Flutter Engine from 60a7bb2353b6 to a6aa5d826649 (2 revisions) (flutter#149627)
  Roll Flutter Engine from ea72558be758 to 60a7bb2353b6 (2 revisions) (flutter#149623)
  Place `flutter_gpu` in the package cache. (flutter#149299)
  Switch to triage-* labels for platform package triage (flutter#149614)
  Roll pub packages (flutter#149617)
  Fixes multi line textfield hint text gets ellipsized (flutter#148423)
  Support failures-only and silent reporters in `flutter test` (flutter#148739)
  [CupertinoActionSheet] Fix overflow of the overscroll section when the user scrolls far (flutter#149542)
  Fix InputDecorator.prefixIcon color when disabled (flutter#149595)
  Added filter callback on dropdown menu (flutter#143939)
  update generated localized message files in the stocks test app (flutter#148741)
  Add a simplified SimpleCascadingMenuApp example (flutter#149147)
  Reland "Prevent LayoutBuilder from rebuilding more than once (flutter#147856)" (flutter#149303)
  Move some benchmarks from MotoG4 to Mokey (flutter#149567)
  Roll Packages from d8e8e8c to 11e192a (2 revisions) (flutter#149596)
  Cleanup triage reports from docs/ (flutter#149545)
  Roll Flutter Engine from d81edf635a9f to ea72558be758 (1 revision) (flutter#149590)
  Roll Flutter Engine from b0f4d7459708 to d81edf635a9f (1 revision) (flutter#149468)
  Roll Flutter Engine from 40b868efcc46 to b0f4d7459708 (1 revision) (flutter#149467)
  ...
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

autosubmit Merge PR when tree becomes green via auto submit App f: cupertino flutter/packages/flutter/cupertino repository framework flutter/packages/flutter repository. See also f: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants