Skip to content

Conversation

@MitchellGoodwin
Copy link
Contributor

@MitchellGoodwin MitchellGoodwin commented Mar 22, 2023

Addresses #102813. Adds a radio widget to Cupertino. Like the checkbox, this will be made adaptable with the Material radio widget.

Here's the radio buttons in the macOS settings:
Screenshot 2023-03-22 at 3 48 58 PM

And here's the Cupertino version in action, including keyboard focus/ interaction:

Screen.Recording.2023-03-22.at.3.49.57.PM.mov

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.
  • All existing and new tests are passing.

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

@flutter-dashboard flutter-dashboard bot added d: api docs Issues with https://api.flutter.dev/ d: examples Sample code and demos documentation f: cupertino flutter/packages/flutter/cupertino repository framework flutter/packages/flutter repository. See also f: labels. c: contributor-productivity Team-specific productivity, code health, technical debt. labels Mar 22, 2023
@MitchellGoodwin MitchellGoodwin marked this pull request as ready for review March 24, 2023 17:31
Copy link
Contributor

@gspencergoog gspencergoog left a comment

Choose a reason for hiding this comment

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

Both of the examples will need tests. See some of the other tests under examples/api/test to see how they're written. They're pretty simple tests just to make sure that the example displays the expected components of the example.

Copy link
Contributor

Choose a reason for hiding this comment

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

You should give this a more specific name: (e.g. 'CuptertinoRadio Example').

Copy link
Contributor

Choose a reason for hiding this comment

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

Try naming the app something like "CupertinoRadioApp" or something to make it match the example.

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// Flutter code sample for [CupertinoRadio].
/// Flutter code sample for [CupertinoRadio].

This should be a doc comment so that IDE users can click on the link to get back to the class the example is attached to.

You might need to move this below the import if the analyzer complains.

Copy link
Contributor

Choose a reason for hiding this comment

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

You can probably get rid of this. The title is not really used on iOS, I don't think. On Android it's used when you pull down the window shade, but that's all.

Copy link
Contributor

Choose a reason for hiding this comment

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

Name this something specific to this example: CupertinoRadioExample or something.

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
/// radio button with the new [groupValue].
/// radio button with a new [groupValue].

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
/// ** See code in examples/api/lib/cupertino/radio/radio.toggleable.0.dart **
/// ** See code in examples/api/lib/cupertino/radio/cupertino_radio.toggleable.0.dart **

Copy link
Contributor

Choose a reason for hiding this comment

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

Is it really a "shadow"? I think I'd call it just a border.

Copy link
Contributor

Choose a reason for hiding this comment

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

You might move this to a named constant at the top of the file.

@MitchellGoodwin MitchellGoodwin force-pushed the create-cupertino-radio branch from 29f8af3 to 783cf86 Compare March 27, 2023 21:47
@MitchellGoodwin MitchellGoodwin force-pushed the create-cupertino-radio branch from 783cf86 to 815fbe3 Compare March 29, 2023 18:32
Copy link
Contributor

@gspencergoog gspencergoog left a comment

Choose a reason for hiding this comment

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

32384589-a60f0e74-c078-11e7-9bc1-e5b5287aea9d

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 30, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 31, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 31, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 31, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 31, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 31, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 31, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 31, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Apr 1, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Apr 1, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Apr 1, 2023
exaby73 pushed a commit to NevercodeHQ/flutter that referenced this pull request Apr 17, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 10, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 10, 2023
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 c: contributor-productivity Team-specific productivity, code health, technical debt. d: api docs Issues with https://api.flutter.dev/ d: examples Sample code and demos 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