Skip to content

Conversation

@victorsanni
Copy link
Contributor

@victorsanni victorsanni commented Jul 21, 2025

@github-actions github-actions bot added framework flutter/packages/flutter repository. See also f: labels. f: cupertino flutter/packages/flutter/cupertino repository labels Jul 21, 2025
@github-actions github-actions bot added the f: material design flutter/packages/flutter/material repository. label Jul 22, 2025
@flutter-dashboard
Copy link

Golden file changes have been found for this pull request. Click here to view and triage (e.g. because this is an intentional change).

If you are still iterating on this change and are not ready to resolve the images on the Flutter Gold dashboard, consider marking this PR as a draft pull request above. You will still be able to view image results on the dashboard, commenting will be silenced, and the check will not try to resolve itself until marked ready for review.

For more guidance, visit Writing a golden file test for package:flutter.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

Changes reported for pull request #172502 at sha fd3da40

@flutter-dashboard flutter-dashboard bot added the will affect goldens Changes to golden files label Jul 22, 2025
@victorsanni
Copy link
Contributor Author

Scuba changes are pretty bad. Maybe we can add it as an optional parameter and have it stick to its current default.

@victorsanni victorsanni marked this pull request as draft July 22, 2025 20:05
@victorsanni victorsanni removed the request for review from MitchellGoodwin July 22, 2025 20:05
@flutter-dashboard
Copy link

This pull request has been changed to a draft. The currently pending flutter-gold status will not be able to resolve until a new commit is pushed or the change is marked ready for review again.

For more guidance, visit Writing a golden file test for package:flutter.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

@github-actions github-actions bot removed the f: material design flutter/packages/flutter/material repository. label Jul 22, 2025
@victorsanni victorsanni changed the title Adjust CupertinoCheckbox padding from 44px to 18px Add CupertinoCheckbox.tapTargetSize Jul 22, 2025
@victorsanni
Copy link
Contributor Author

This is tricky because if we expose the tapTargetSize, there is weird behavior if they supply a size <18px:

Screenshot 2025-09-03 at 11 30 10 AM

The solutions I think could work here are:

  • Hard break everyone to move to 18px since checkboxes aren't recommended on mobile anyways
  • add the 18px checkbox as a new constructor (CupertinoCheckbox.small/CupertinoCheckbox.large or CupertinoCheckbox.mobile/CupertinoCheckbox.desktop)
  • add the tapTargetSize property and assert a minimum size of 18x18 px

Which of these should we proceed with @MitchellGoodwin?

@MitchellGoodwin
Copy link
Contributor

My vote is for

add the tapTargetSize property and assert a minimum size of 18x18 px

Besides visual appearance changes, tap targets are in the realm of accessibility so I'm a little nervous to make breaking changes when it comes to that. Also, rather than 18x18, it might make sense to make the minimum 14x14. 18 is the width of the radio, 14 is the width of the checkbox. I don't know if we need to enforce having a tap target bigger than the checkbox.

Right now the width of the checkbox is set by a static value. I think the min TapTargetSize and the width should point to the same value.

I think we can merge this in and then consider a breaking change after. I think we might want to consider having tapTargetSize default to it's current value on mobile, and to a lower default on all other platforms. That makes the most sense to me, but would be breaking on desktop and web.

@victorsanni
Copy link
Contributor Author

I think we might want to consider having tapTargetSize default to it's current value on mobile, and to a lower default on all other platforms.

That's an option I didn't consider. And might help with the google tests as well. Plus, it means we don't have to increase the API surface.

@MitchellGoodwin
Copy link
Contributor

Another thing to consider is that it's common to adjust the size of the checkbox with Transform.scale, so whatever we do we need to make sure it works as expected with that.

@victorsanni victorsanni marked this pull request as ready for review September 3, 2025 19:27
@victorsanni victorsanni changed the title Add CupertinoCheckbox.tapTargetSize Adjust default CupertinoCheckbox size on desktop Sep 3, 2025
TargetPlatform.fuchsia => const Size.square(kMinInteractiveDimensionCupertino),
TargetPlatform.macOS ||
TargetPlatform.linux ||
TargetPlatform.windows => const Size.square(CupertinoCheckbox.width),
Copy link
Contributor

Choose a reason for hiding this comment

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

CupertinoCheckbox.width is 14 while the size tested in the original issue is 18, which looks pretty good. Will 14 be too small?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, I think the issue has 18px because of the default macOS padding. But it's probably best to leave any additional padding customizations to the user.

Copy link
Contributor

@dkwingsmt dkwingsmt left a comment

Choose a reason for hiding this comment

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

I think a better way is to expose a size property that applies the platform-dependent default size in this PR. What do you think?

@victorsanni
Copy link
Contributor Author

I think a better way is to expose a size property that applies the platform-dependent default size in this PR. What do you think?

It gets weird because the size here doesn't actually affect the size of the checkbox, just the area that, when tapped, toggles the checkbox.

Copy link
Contributor

@dkwingsmt dkwingsmt left a comment

Choose a reason for hiding this comment

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

LGTM, thank you

Copy link
Contributor

@MitchellGoodwin MitchellGoodwin left a comment

Choose a reason for hiding this comment

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

LGTM with one nit on the documentation

/// ([CupertinoSwitch] in Flutter) instead, or to find a creative custom
/// solution.
///
/// The checkbox has a default size of 14-by-14 pixels on desktop devices and
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: I think this should clarify that the size difference is only the tap target area and layout padding. The actual visual checkbox itself is 14-by-14 on all platforms with this change.

@victorsanni victorsanni added the autosubmit Merge PR when tree becomes green via auto submit App label Sep 9, 2025
@auto-submit auto-submit bot added this pull request to the merge queue Sep 9, 2025
Merged via the queue into flutter:master with commit 53f8432 Sep 9, 2025
79 checks passed
@flutter-dashboard flutter-dashboard bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Sep 9, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 9, 2025
@victorsanni victorsanni deleted the checkbox-padding branch September 9, 2025 19:30
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 10, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 10, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 10, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 11, 2025
auto-submit bot pushed a commit to flutter/packages that referenced this pull request Sep 11, 2025
flutter/flutter@a082096...5a6a1bf

2025-09-11 [email protected] Roll Dart SDK from 42045594dbc6 to f7d6a4732ab0 (2 revisions) (flutter/flutter#175222)
2025-09-11 [email protected] Roll Skia from 00c8b3f69de9 to ead9277819fc (2 revisions) (flutter/flutter#175221)
2025-09-11 [email protected] Roll Skia from 4a8817a48b25 to 00c8b3f69de9 (3 revisions) (flutter/flutter#175213)
2025-09-11 [email protected] Roll Fuchsia Linux SDK from 10x-JGF5zuuW8ik4K... to 1pTB3J5rn4YYugylf... (flutter/flutter#175210)
2025-09-11 [email protected] Roll Dart SDK from 1de2289e49fe to 42045594dbc6 (1 revision) (flutter/flutter#175203)
2025-09-10 [email protected] Roll Skia from cbb0388767d2 to 4a8817a48b25 (4 revisions) (flutter/flutter#175202)
2025-09-10 [email protected] Roll Skia from 55c9d697da52 to cbb0388767d2 (7 revisions) (flutter/flutter#175197)
2025-09-10 [email protected] Roll Dart SDK from f446144fb7c9 to 1de2289e49fe (3 revisions) (flutter/flutter#175192)
2025-09-10 [email protected] Roll Skia from c3a3d1e47699 to 55c9d697da52 (4 revisions) (flutter/flutter#175190)
2025-09-10 [email protected] Roll Packages from 2d651b2 to 03598e7 (5 revisions) (flutter/flutter#175185)
2025-09-10 [email protected] Roll Skia from 36f3c3fbec19 to c3a3d1e47699 (2 revisions) (flutter/flutter#175181)
2025-09-10 [email protected] Marks Linux plugin_test_android_standard to be unflaky (flutter/flutter#175163)
2025-09-10 [email protected] Roll Skia from 97497ee065e4 to 36f3c3fbec19 (5 revisions) (flutter/flutter#175178)
2025-09-10 [email protected] Marks Windows plugin_test_android_variants to be unflaky (flutter/flutter#175167)
2025-09-10 [email protected] Marks Linux plugin_test_android_variants to be unflaky (flutter/flutter#175162)
2025-09-10 [email protected] Marks Windows plugin_test_android_standard to be unflaky (flutter/flutter#175168)
2025-09-10 [email protected] Marks Mac plugin_test_android_standard to be unflaky (flutter/flutter#175166)
2025-09-10 [email protected] Marks Mac plugin_test_android_variants to be unflaky (flutter/flutter#175165)
2025-09-10 [email protected] [shell] Fix engineId not being set after hot restart (flutter/flutter#174451)
2025-09-10 [email protected] Roll Skia from 6acb8b29b60e to 97497ee065e4 (1 revision) (flutter/flutter#175152)
2025-09-10 [email protected] Roll Skia from 127786500ad0 to 6acb8b29b60e (1 revision) (flutter/flutter#175148)
2025-09-10 [email protected] Roll Skia from 04513dfdf517 to 127786500ad0 (2 revisions) (flutter/flutter#175145)
2025-09-09 [email protected] Roll Fuchsia Linux SDK from m7Qmvj5wtfPlMA8i8... to 10x-JGF5zuuW8ik4K... (flutter/flutter#175140)
2025-09-09 [email protected] Roll Skia from 416b3b42ece2 to 04513dfdf517 (5 revisions) (flutter/flutter#175137)
2025-09-09 [email protected] Roll Skia from 19ba56dde579 to 416b3b42ece2 (1 revision) (flutter/flutter#175134)
2025-09-09 [email protected] Adjust default CupertinoCheckbox size on desktop (flutter/flutter#172502)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages
Please CC [email protected],[email protected] on the revert to ensure that a human
is aware of the problem.

To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
mboetger pushed a commit to mboetger/flutter that referenced this pull request Sep 18, 2025
Fixes [CupertinoCheckbox padding on MacOS is excessive, is applying iOS
standards](flutter#165140)
Jaineel-Mamtora pushed a commit to Jaineel-Mamtora/flutter_forked that referenced this pull request Sep 24, 2025
Fixes [CupertinoCheckbox padding on MacOS is excessive, is applying iOS
standards](flutter#165140)
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 12, 2025
lucaantonelli pushed a commit to lucaantonelli/flutter that referenced this pull request Nov 21, 2025
Fixes [CupertinoCheckbox padding on MacOS is excessive, is applying iOS
standards](flutter#165140)
FeodorFitsner added a commit to flet-dev/flet that referenced this pull request Nov 23, 2025
Introduces a 'spacing' property to CupertinoCheckbox for customizing the space between the checkbox and its label. Updates Dart and Python implementations, integration tests, and golden images to reflect this change. Also refactors test cases for improved clarity and coverage.

Flutter change: flutter/flutter#172502
FeodorFitsner added a commit to flet-dev/flet that referenced this pull request Nov 23, 2025
* Update Flutter version to 3.38.2 and dependencies

Bumped the required Flutter version to 3.38.2 in .fvmrc and build.py. Updated several Dart dependencies in pubspec.lock and removed path_provider_foundation from macOS plugin registration to reflect dependency changes.

* Update ci.yml

* Update Flutter cache key format in CI workflow

Revised the cache-key and pub-cache-key formats in the CI workflow to use placeholders (:os:, :channel:, :version:, :hash:) instead of GitHub Actions variables. This may improve cache management and compatibility with the flutter-fvm-config-action.

* Update Flutter cache keys to include architecture

Modified cache-key and pub-cache-key in CI and macOS integration workflows to include architecture. This improves cache granularity and prevents issues when running workflows on different architectures.

* Disable cache in Flutter FVM setup actions

Set 'cache' to false in the flutter-fvm-config-action setup steps for both CI and macOS integration test workflows. This change disables caching to potentially resolve issues related to cache usage or improve workflow reliability.

* Add Flutter caching to CI workflows

Introduces a cache step for Flutter in both CI and macOS integration test workflows using actions/cache. This improves build performance by reusing Flutter dependencies between runs.

* Add step to print Flutter cache outputs in CI

Introduces a step in both CI and macOS integration test workflows to print the Flutter cache path and key outputs. This aids in debugging and verifying cache configuration during workflow runs.

* Update Flutter setup in CI workflows

Replaces kuhnroyal/flutter-fvm-config-action/setup with subosito/flutter-action for Flutter setup in both ci.yml and macos-integration-tests.yml. Adds a separate step to configure Flutter version using .fvmrc and removes printing of Flutter outputs.

* Add spacing option to CupertinoCheckbox label

Introduces a 'spacing' property to CupertinoCheckbox for customizing the space between the checkbox and its label. Updates Dart and Python implementations, integration tests, and golden images to reflect this change. Also refactors test cases for improved clarity and coverage.

Flutter change: flutter/flutter#172502

* Update golden images for macOS material tests

Refreshed golden images for submenu button and time picker integration tests on macOS to reflect recent UI or rendering changes.

* Improve mouse gesture handling in FlutterWidgetTester

Introduces proper mouse pointer exit before tap, long press, and text entry actions to avoid lingering gestures. Updates mouse hover logic to manage gesture lifecycle. Also updates navigation bar test destinations and golden images for macOS time picker and navigation bar.

* Update dropdown theme_1.png golden image

Replaces the macOS dropdown theme_1.png golden image in integration tests to reflect updated visual output or design changes.

* Update golden image for macOS menu bar test

Replaces the menu_bar_basic_open.png golden image used in integration tests for the macOS menu bar control. This ensures test accuracy with the latest UI changes.

* Remove unnecessary mouse exit calls in FlutterWidgetTester

Eliminated redundant _mouseExit() calls before tap, longPress, and enterText actions in FlutterWidgetTester to streamline test interactions. Also updated golden image for navigation bar theme on macOS.

* Add 'persist' option to SnackBar control

Introduces a 'persist' property to the SnackBar control in Dart and Python, allowing developers to specify whether the snack bar should remain visible after the timeout. Updates documentation and example usage to reflect the new option.

Breaking change: https://docs.flutter.dev/release/breaking-changes/snackbar-with-action-behavior-update

* Make spacing property optional in CupertinoCheckbox

Changed the type of the spacing property from Number to Optional[Number] in CupertinoCheckbox to allow for None values and improve flexibility.
FeodorFitsner added a commit to flet-dev/flet that referenced this pull request Nov 23, 2025
…ied/intelligent `src` prop (#5817)

* initial commit

* update examples

* tests

* `enable_merge_paths=False` by default

* improve URL and file path handling in image resolution

* `Slider`: pass value to change event

* Refactor image source handling in controls and improve image provider methods

* more tests

* Fix image source parameter in canvas test

* fix #5615: Refactor DashedStrokePattern validation and improve segments handling

* Add tip for scrollable content in bottom sheet documentation

* Refactor asset source resolution and minor code cleanup

Replaced ResolvedAssetSource.from(...get('src')) with getSrc('src') in audio and lottie Dart files for improved asset source resolution. Cleaned up argument formatting and improved readability in lottie.dart. Updated brush.py to use src_bytes instead of src for file saving. Removed unused page argument and debug print statements from image_switch_buffered.py.

* Upgrade to Flutter 3.38.2 (#5851)

* Update Flutter version to 3.38.2 and dependencies

Bumped the required Flutter version to 3.38.2 in .fvmrc and build.py. Updated several Dart dependencies in pubspec.lock and removed path_provider_foundation from macOS plugin registration to reflect dependency changes.

* Update ci.yml

* Update Flutter cache key format in CI workflow

Revised the cache-key and pub-cache-key formats in the CI workflow to use placeholders (:os:, :channel:, :version:, :hash:) instead of GitHub Actions variables. This may improve cache management and compatibility with the flutter-fvm-config-action.

* Update Flutter cache keys to include architecture

Modified cache-key and pub-cache-key in CI and macOS integration workflows to include architecture. This improves cache granularity and prevents issues when running workflows on different architectures.

* Disable cache in Flutter FVM setup actions

Set 'cache' to false in the flutter-fvm-config-action setup steps for both CI and macOS integration test workflows. This change disables caching to potentially resolve issues related to cache usage or improve workflow reliability.

* Add Flutter caching to CI workflows

Introduces a cache step for Flutter in both CI and macOS integration test workflows using actions/cache. This improves build performance by reusing Flutter dependencies between runs.

* Add step to print Flutter cache outputs in CI

Introduces a step in both CI and macOS integration test workflows to print the Flutter cache path and key outputs. This aids in debugging and verifying cache configuration during workflow runs.

* Update Flutter setup in CI workflows

Replaces kuhnroyal/flutter-fvm-config-action/setup with subosito/flutter-action for Flutter setup in both ci.yml and macos-integration-tests.yml. Adds a separate step to configure Flutter version using .fvmrc and removes printing of Flutter outputs.

* Add spacing option to CupertinoCheckbox label

Introduces a 'spacing' property to CupertinoCheckbox for customizing the space between the checkbox and its label. Updates Dart and Python implementations, integration tests, and golden images to reflect this change. Also refactors test cases for improved clarity and coverage.

Flutter change: flutter/flutter#172502

* Update golden images for macOS material tests

Refreshed golden images for submenu button and time picker integration tests on macOS to reflect recent UI or rendering changes.

* Improve mouse gesture handling in FlutterWidgetTester

Introduces proper mouse pointer exit before tap, long press, and text entry actions to avoid lingering gestures. Updates mouse hover logic to manage gesture lifecycle. Also updates navigation bar test destinations and golden images for macOS time picker and navigation bar.

* Update dropdown theme_1.png golden image

Replaces the macOS dropdown theme_1.png golden image in integration tests to reflect updated visual output or design changes.

* Update golden image for macOS menu bar test

Replaces the menu_bar_basic_open.png golden image used in integration tests for the macOS menu bar control. This ensures test accuracy with the latest UI changes.

* Remove unnecessary mouse exit calls in FlutterWidgetTester

Eliminated redundant _mouseExit() calls before tap, longPress, and enterText actions in FlutterWidgetTester to streamline test interactions. Also updated golden image for navigation bar theme on macOS.

* Add 'persist' option to SnackBar control

Introduces a 'persist' property to the SnackBar control in Dart and Python, allowing developers to specify whether the snack bar should remain visible after the timeout. Updates documentation and example usage to reflect the new option.

Breaking change: https://docs.flutter.dev/release/breaking-changes/snackbar-with-action-behavior-update

* Make spacing property optional in CupertinoCheckbox

Changed the type of the spacing property from Number to Optional[Number] in CupertinoCheckbox to allow for None values and improve flexibility.

---------

Co-authored-by: Feodor Fitsner <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

f: cupertino flutter/packages/flutter/cupertino repository framework flutter/packages/flutter repository. See also f: labels. will affect goldens Changes to golden files

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CupertinoCheckbox padding on MacOS is excessive, is applying iOS standards

3 participants