Improve `CupertinoRadio` fidelity #149703

victorsanni · 2024-06-05T00:47:17Z

Adds the following:

Darkens when pressed in light mode
Lightens when pressed in dark mode.
Tests that confirm CupertinoRadio is focusable and has correct focus colors
Tests that confirm CupertinoRadio uses correct default active/inactive/fill colors
Same look in disabled vs. enabled states as native macOS:

Native macOS	Flutter Before	Flutter After

Same look of an unselected radio button in dark mode as native macOS:

Native light mode	Flutter before light mode	Flutter after light mode	Native dark mode	Flutter before dark mode	Flutter after dark mode

Light mode (with focus highlight)

Native light mode	Flutter before light mode	Flutter after light mode

Dark mode

Native dark mode	Flutter before dark mode	Flutter after dark mode

Disabled light mode

Native	Flutter before	Flutter after

Disabled dark mode

Native	Flutter before	Flutter after

CupertinoRadio is missing a tristate/mixed state, but Apple's latest HIG specs discourages its use.

Fixes #151994

Pre-launch Checklist

I read the Contributor Guide and followed the process outlined there for submitting PRs.
I read the Tree Hygiene wiki page, which explains my responsibilities.
I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
I signed the CLA.
I listed at least one issue that this PR fixes in the description above.
I updated/added relevant documentation (doc comments with ///).
I added new tests to check the change I am making, or this PR is test-exempt.
I followed the breaking change policy and added Data Driven Fixes where supported.
All existing and new tests are passing.

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

flutter-dashboard · 2024-07-27T01:23:06Z

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 #149703 at sha 4bc506b

flutter-dashboard · 2024-07-29T19:25:31Z

Golden file changes are available for triage from new commit, Click here to view.

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 #149703 at sha 1b5ba82

justinmc

Thanks for all of the great screenshots in the description! LGTM 👍

packages/flutter/test/cupertino/radio_test.dart

dkwingsmt · 2024-07-30T21:05:22Z

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

+        final Paint outerPaint = Paint()
+        // In dark mode, the outer color of an active radio is slightly darker.
+          ..color = isActive
+            ? (isDefaultSelected && brightness == Brightness.dark ? _kDarkModeOuterColor : activeColor)


The logic around isDefaultSelected feels pretty weird to me. For example, if the fillColor is customized, then suddenly the active outer color in dark mode changes from _kDarkModeOuterColor to activeColor, which is very surprising.

Is there a reason why a more solid way isn't feasible? For example, can we make the active color a CupertinoDynamicColor?

Yes. That's very true. I've refactored to do this outside the painter instead, using CupertinoDynamicColor to resolve the default dark mode color. Thanks for pointing this out.

flutter-dashboard · 2024-07-30T22:29:11Z

Golden file changes are available for triage from new commit, Click here to view.

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 #149703 at sha bf9e75a

Manual roll requested by stuartmorgan@google.com flutter/flutter@f10a497...cbfb222 2024-08-04 engine-flutter-autoroll@skia.org Roll Flutter Engine from 980577996f38 to 16012e2f8ccd (1 revision) (flutter/flutter#152824) 2024-08-03 engine-flutter-autoroll@skia.org Roll Flutter Engine from 2a51c687fd40 to 980577996f38 (1 revision) (flutter/flutter#152821) 2024-08-03 engine-flutter-autoroll@skia.org Roll Flutter Engine from 4c868ee85616 to 2a51c687fd40 (2 revisions) (flutter/flutter#152818) 2024-08-03 engine-flutter-autoroll@skia.org Roll Flutter Engine from afb7007298cc to 4c868ee85616 (2 revisions) (flutter/flutter#152814) 2024-08-03 zanderso@users.noreply.github.com Fix device_os requested in linux_build_test tests (flutter/flutter#152808) 2024-08-03 engine-flutter-autoroll@skia.org Roll Flutter Engine from 516235e4456b to afb7007298cc (3 revisions) (flutter/flutter#152804) 2024-08-03 zanderso@users.noreply.github.com Fix misunderstanding of properties vs. drone_dimensions in Linux_build_tests (flutter/flutter#152796) 2024-08-03 engine-flutter-autoroll@skia.org Roll Flutter Engine from 3c9d7e3f7c02 to 516235e4456b (3 revisions) (flutter/flutter#152790) 2024-08-03 victorsanniay@gmail.com Improve `CupertinoRadio` fidelity (flutter/flutter#149703) 2024-08-03 engine-flutter-autoroll@skia.org Roll Flutter Engine from 353c6b237b78 to 3c9d7e3f7c02 (3 revisions) (flutter/flutter#152777) 2024-08-02 gspencergoog@users.noreply.github.com Fix handling of `iconSize` and `iconColor` defaults for `ButtonStyleButton` subclasses. (flutter/flutter#143501) 2024-08-02 matanlurey@users.noreply.github.com Use print logging on LUCI. (flutter/flutter#152776) 2024-08-02 zanderso@users.noreply.github.com Reland: Shift Linux_build_test tests from MotoG4 to mokey (flutter/flutter#152756) 2024-08-02 greg@zulip.com Write more on Animation and related docs (flutter/flutter#150727) 2024-08-02 pateltirth454@gmail.com Quick Grammar Fixes (flutter/flutter#152744) 2024-08-02 engine-flutter-autoroll@skia.org Roll Flutter Engine from 077b6f057b69 to 353c6b237b78 (3 revisions) (flutter/flutter#152762) 2024-08-02 anuragr9847@gmail.com [SliderTheme] Fix markdown links for api doc images (flutter/flutter#152748) 2024-08-02 tugorez@users.noreply.github.com Make the App's title optional on web (flutter/flutter#152003) 2024-08-02 32538273+ValentinVignal@users.noreply.github.com Add tests for scaffold messenger state (flutter/flutter#152735) 2024-08-02 srawlins@google.com Ignore both unused_element and unused_element_parameter (flutter/flutter#152689) 2024-08-02 parlough@gmail.com Update dartdoc to 8.0.12 to fix focusing search field (flutter/flutter#151576) 2024-08-02 rexios@rexios.dev [wiki] Remove outdated warning about stale coverage data (flutter/flutter#152560) 2024-08-02 engine-flutter-autoroll@skia.org Roll Packages from 27896d1 to cc9ff47 (8 revisions) (flutter/flutter#152754) 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 rmistry@google.com,stuartmorgan@google.com 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

Adds the following: * Darkens when pressed in light mode * Lightens when pressed in dark mode. * Tests that confirm `CupertinoRadio` is focusable and has correct focus colors * Tests that confirm `CupertinoRadio` uses correct default active/inactive/fill colors * Same look in disabled vs. enabled states as native macOS: | Native macOS | Flutter Before | Flutter After | --- | --- | --- | | <img width="50" alt="radio native" src="https://github.com/flutter/flutter/assets/77553258/27c8c27e-f0dc-4ad7-a8c2-361ae8b437bb"> | <img width="50" alt="flutter radio before" src="https://github.com/flutter/flutter/assets/77553258/580d9c4b-0f0d-457e-851f-73450738ee16"> | <img width="50" alt="flutter radio after" src="https://github.com/flutter/flutter/assets/77553258/da6ae21b-87f8-45d8-a2d2-da70ff4853a1"> | * Same look of an unselected radio button in dark mode as native macOS: | Native light mode | Flutter before light mode | Flutter after light mode | Native dark mode | Flutter before dark mode | Flutter after dark mode --- | --- | --- | --- | --- | --- | | <img width="23" alt="native radio light" src="https://github.com/flutter/flutter/assets/77553258/b52fc18b-e10d-4205-b10b-1536fbbf1ca0"> | <img width="23" alt="flutter radio after light" src="https://github.com/flutter/flutter/assets/77553258/54294523-8254-479c-b668-77927a8295f1"> | <img width="23" alt="flutter radio light" src="https://github.com/flutter/flutter/assets/77553258/8472deee-e5ce-4d39-9207-d788ad7f34f4"> | <img width="23" alt="native radio dark" src="https://github.com/flutter/flutter/assets/77553258/44143099-6ab4-4fb8-8a94-ebb1386022c9"> | <img width="23" alt="flutter radio before dark" src="https://github.com/flutter/flutter/assets/77553258/3411d9fb-fc7f-4b20-86a5-34fda167d5b9"> | <img width="23" alt="flutter radio dark" src="https://github.com/flutter/flutter/assets/77553258/39ea3649-142e-43ad-9681-24e1216e0987"> | ## Light mode (with focus highlight) | Native light mode | Flutter before light mode | Flutter after light mode | --- | --- | --- | | <img width="70" alt="native radio light mode" src="https://github.com/user-attachments/assets/914b9f1f-5819-4c5b-8739-8498a72b337f"> | <img width="70" alt="radio flutter focus before" src="https://github.com/user-attachments/assets/3129fca3-3310-4b2b-bcf3-98aa8f049911"> | <img width="70" alt="radio flutter focus after" src="https://github.com/user-attachments/assets/7a2089d9-b2b5-4ff0-9db9-444455301146"> | ## Dark mode | Native dark mode | Flutter before dark mode | Flutter after dark mode | --- | --- | --- | | <img width="70" alt="native radio dark mode" src="https://github.com/user-attachments/assets/4da3c055-ce89-4f37-8fcd-d4cbbc4031a0"> | <img width="70" alt="flutter before radio dark mode" src="https://github.com/user-attachments/assets/36b5f36a-f1d9-4c32-8493-3533a749cf5d"> | <img width="70" alt="flutter radio dark mode after" src="https://github.com/user-attachments/assets/28828e01-bb2f-4217-9756-2766be3919a6"> | ## Disabled light mode | Native | Flutter before | Flutter after | --- | --- | --- | | <img width="120" alt="light disabled radio native" src="https://github.com/user-attachments/assets/bf6d2561-5dcf-4882-afac-6b639fa949b0"> | <img width="120" alt="Screenshot 2024-07-30 at 3 13 30 PM" src="https://github.com/user-attachments/assets/3efc978c-fa58-44e8-877a-ea29778ea384"> | <img width="120" alt="light disabled radio flutter after" src="https://github.com/user-attachments/assets/b2c2e30a-cb8d-40d0-aa6f-75a98caa4829"> | ## Disabled dark mode | Native | Flutter before | Flutter after | --- | --- | --- | | <img width="120" alt="dark disabled radio native" src="https://github.com/user-attachments/assets/feedccc7-9802-4b0c-8038-c9eb771b0eb0"> | <img width="120" alt="Screenshot 2024-07-30 at 3 13 30 PM" src="https://github.com/user-attachments/assets/6d2f03f7-7216-4850-8c4f-f79ae05bb9da"> | <img width="136" alt="dark disabled radio flutter after" src="https://github.com/user-attachments/assets/5e03d4fc-4b8e-4518-b429-6bb58f6d988d"> | `CupertinoRadio` is missing a tristate/mixed state, but [Apple's latest HIG specs discourages its use](https://developer.apple.com/design/human-interface-guidelines/toggles#Radio-buttons). Fixes flutter#151994 ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [ ] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord].  [Contributor Guide]: https://github.com/flutter/flutter/wiki/Tree-hygiene#overview [Tree Hygiene]: https://github.com/flutter/flutter/wiki/Tree-hygiene [test-exempt]: https://github.com/flutter/flutter/wiki/Tree-hygiene#tests [Flutter Style Guide]: https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo [Features we expect every widget to implement]: https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/wiki/Tree-hygiene#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/wiki/Chat [Data Driven Fixes]: https://github.com/flutter/flutter/wiki/Data-driven-Fixes --------- Co-authored-by: Kate Lovett <katelovett@google.com>

Changes in painter with tests

6aea36b

github-actions bot added framework flutter/packages/flutter repository. See also f: labels. f: cupertino flutter/packages/flutter/cupertino repository labels Jun 5, 2024

Minor test name changes

5846899

victorsanni force-pushed the improve-cupertino-radio-fidelity branch from 803e4ef to 5846899 Compare June 5, 2024 01:00

Minor test indent changes

2492668

victorsanni added 3 commits July 26, 2024 16:22

Change default color/shape/size tests to goldens

2659814

Merge branch 'master' into improve-cupertino-radio-fidelity

13c2136

Fix golden tests formatting

4bc506b

flutter-dashboard bot added the will affect goldens Changes to golden files label Jul 27, 2024

victorsanni added 3 commits July 29, 2024 11:43

Add outer border for disabled selected radio + small refactors

0a05556

Minor formatting updates

569e391

Merge branch 'master' into improve-cupertino-radio-fidelity

1b5ba82

justinmc approved these changes Jul 30, 2024

View reviewed changes

dkwingsmt reviewed Jul 30, 2024

View reviewed changes

victorsanni added 3 commits July 30, 2024 14:51

Refactor to match CupertinoCheckbox behavior

f9556a8

Merge branch 'master' into improve-cupertino-radio-fidelity

df3218f

Wrap goldens with RepaintBoundary

bf9e75a

victorsanni added 3 commits August 2, 2024 04:11

Update radio.dart

e325401

Merge branch 'master' into improve-cupertino-radio-fidelity

8fc0119

Merge branch 'master' into improve-cupertino-radio-fidelity

55ee814

victorsanni merged commit 6b73de2 into flutter:master Aug 3, 2024
71 of 72 checks passed

victorsanni deleted the improve-cupertino-radio-fidelity branch August 3, 2024 01:32

engine-flutter-autoroll mentioned this pull request Aug 4, 2024

Manual roll Flutter from f10a497e4abd to cbfb22244697 (23 revisions) flutter/packages#7298

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 4, 2024

6b73de2 Improve CupertinoRadio fidelity (flutter/flutter#149703)

adf2d3a

engine-flutter-autoroll mentioned this pull request Aug 4, 2024

Roll Flutter from f10a497e4abd to cbfb22244697 (23 revisions) flutter/packages#7299

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 4, 2024

6b73de2 Improve CupertinoRadio fidelity (flutter/flutter#149703)

6d5c8d4

Piinks mentioned this pull request Aug 9, 2024

Flutter Framework Team: Notable Commits #121415

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 11, 2024

6b73de2 Improve CupertinoRadio fidelity (flutter/flutter#149703)

4575436

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 12, 2024

6b73de2 Improve CupertinoRadio fidelity (flutter/flutter#149703)

bb87418

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Improve `CupertinoRadio` fidelity #149703

Improve `CupertinoRadio` fidelity #149703

Uh oh!

victorsanni commented Jun 5, 2024 •

edited

Loading

Uh oh!

flutter-dashboard bot commented Jul 27, 2024

Uh oh!

flutter-dashboard bot commented Jul 29, 2024

Uh oh!

justinmc left a comment

Uh oh!

Uh oh!

dkwingsmt Jul 30, 2024

Uh oh!

victorsanni Jul 30, 2024

Uh oh!

flutter-dashboard bot commented Jul 30, 2024

Uh oh!

Uh oh!

Uh oh!

Improve CupertinoRadio fidelity #149703

Improve CupertinoRadio fidelity #149703

Uh oh!

Conversation

victorsanni commented Jun 5, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Light mode (with focus highlight)

Dark mode

Disabled light mode

Disabled dark mode

Pre-launch Checklist

Uh oh!

flutter-dashboard bot commented Jul 27, 2024

Uh oh!

flutter-dashboard bot commented Jul 29, 2024

Uh oh!

justinmc left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

dkwingsmt Jul 30, 2024

Choose a reason for hiding this comment

Uh oh!

victorsanni Jul 30, 2024

Choose a reason for hiding this comment

Uh oh!

flutter-dashboard bot commented Jul 30, 2024

Uh oh!

Uh oh!

Uh oh!

Improve `CupertinoRadio` fidelity #149703

Improve `CupertinoRadio` fidelity #149703

victorsanni commented Jun 5, 2024 •

edited

Loading