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

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

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.

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

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?

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.

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.

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.

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.

I see that Material has a MaterialTapTargetSize that does a similar job. It's actually an enum, which does exactly what your current switch statement does. That being said, I think the enum is a bad choice (it was added in 2018 anyway) but we can call this new property tapTargetSize and make it a Size.