Adds radio group widget #167363

chunhtai · 2025-04-17T17:22:46Z

This pr introduce a new widget RadioGroup

It does two things:

maintain the group value and provide a simpler api to get the group value
Provide semantics and keyboard navigation matching https://www.w3.org/WAI/ARIA/apg/patterns/radio/

If for some reason, one only wants to have (2), they can use RadioGroupPolicy.

Before this change developers have to implement

int? selection;

  @override
  Widget build(BuildContext context) {
    return Column(
      children: <Widget>[
        Radio<int>(
            value: 0,
            groupValue: selection,
            onChanged: (int? value) {
              setState(() {
                selection = value;
              });
            },
          ),
        Radio<int>(
            value: 1,
            groupValue: selection,
            onChanged: (int? value) {
              setState(() {
                selection = value;
              });
            },
          ),
      ],
    );
  }

After this change

  @override
  Widget build(BuildContext context) {
    return RadioGroup<int>(
      onChange: (int? value) => print(value)
      child: Column(
        children: <Widget>[
          Radio<int>(value: 0),
          Radio<int>(value: 1),
        ],
      ),
    );
  }

They also get keyboard behaviors match Accessibility standard

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.

chunhtai · 2025-04-18T21:48:16Z

The pr right now only support material radio, or cupertino radio built as a result of Radio.adaptive. If we want to support CupertinoRadio, we have to move some of the Radio logic and RadioGroup to the widget layer. that is something that we can do as a follow up

Piinks

First impression looking at that code snippet ➡️ 💯
Really nice simplification of the API, like seriously.

IMO we should just deprecate the old way - unless is there a way someone could be using Radios that is not compatible with RadioGroup?
We won't be able to assert it is configured right until runtime, and the code is so much nicer in this new way you have designed - and more accessible! I don't think dart fix will be able to help here, so a migration guide would also be helpful.

Piinks · 2025-04-18T22:18:06Z

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

+/// same value type. To use this widget, the [Radio.groupValue] must leave as
+/// null. Otherwise, an assertion error is thrown.


Suggested change

/// same value type. To use this widget, the [Radio.groupValue] must leave as

/// null. Otherwise, an assertion error is thrown.

/// same value type. To use this widget, the [Radio.groupValue] must remain

/// null, for the RadioGroup to manage. Otherwise, an assertion error is thrown.

Piinks · 2025-04-18T22:18:18Z

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

+/// same value type. To use this widget, the [Radio.groupValue] must leave as
+/// null. Otherwise, an assertion error is thrown.
+///
+/// This widget internal builds a [RadioGroupPolicy] to provide an accessible


Suggested change

/// This widget internal builds a [RadioGroupPolicy] to provide an accessible

/// This widget internally builds a [RadioGroupPolicy] to provide an accessible

Piinks · 2025-04-18T22:20:59Z

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

+
+/// A container that provides semantics and keyboard navigation.
+///
+/// This widget treats all [Radio]s in the subtree with the same generic type


Will it assert this statement?

Piinks · 2025-04-18T22:23:15Z

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

    required this.value,
-    required this.groupValue,
-    required this.onChanged,
+    this.groupValue,


Should we deprecate groupValue and really encourage folks to migrate to RadioGroup so that folks are using best practices for accessibility?
The fact that this also reduces verbosity is a huge win.

Piinks · 2025-04-18T22:25:20Z

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

+    _group = context.dependOnInheritedWidgetOfExactType<_RadioGroupStateScope<T>>()?.state;
+    assert(
+      _group == null || widget.groupValue == null,
+      'A RadioGroup can not wrap Radio widget with a non null groupValue. '


This message was a little confusing, but we can revisit - I know you asked for early feedback.

Piinks · 2025-04-18T22:27:09Z

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

  const Radio({
    super.key,
    required this.value,
-    required this.groupValue,


For where we assert against the new and old API choice, I think some of them are not yet considering that both groupValue and onChanged are required for the old path - but I did not look too closely since this is just an initial pass. We can check later.

justinmc

Trying to think of edge cases:

What about programmatically selecting a radio button? Especially after the first build, where initialValue gets more complicated.
What if the developer wants to do some calculation at the time that a radio button is tapped before deciding whether to select it or not?
Are we sure that selecting multiple radio buttons is completely disallowed? Maybe that's a good thing actually.

Typically I like the purely declarative approach where the app developer controls which button is selected by calling setState (similar to React). The old way did follow this approach even if it was verbose and a little awkward. The new way does not if I'm not mistaken, because a radio button will automatically be checked when it is tapped, right? So my instincts make me wary that the new way will cause trouble in certain situations.

Maybe we can do a purely declarative RadioGroup and get the best of both worlds? Something like:

  int? selection;

  @override
  Widget build(BuildContext context) {
    return RadioGroup<int>(
      value: selection,
      onChange: (int? value) {
        setState(() {
          selection = value;
        });
      child: Column(
        children: <Widget>[
          Radio<int>(value: 0),
          Radio<int>(value: 1),
        ],
      ),
    );
  }

justinmc · 2025-04-18T22:55:44Z

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

+    : super();
+
+  /// The initial selection on the [Radio] of the matching [Radio.value].
+  final T? initialGroupValue;


This pattern makes me hesitate, even though I know we've done it elsewhere in the framework. I worry that there are cases where it gets confusing whether an initialValue should be applied or not, like rebuilding the widget, changing the initialValue after user interaction, moving the widget, etc.

chunhtai · 2025-04-22T19:28:59Z

Maybe we can do a purely declarative RadioGroup and get the best of both worlds?

I like the idea, I do think this not only makes it more flexible but also simply some of the implementation. I will make the change.

Are we sure that selecting multiple radio buttons is completely disallowed? Maybe that's a good thing actually.

Not according to material guide, but if they really want to they can do something like

  child: RadioGroup<int>(
      onChange: (int? value) => print(value)
      child: Column(
        children: <Widget>[
          Radio<int>(value: 0),
          Radio<int>(value: 1),
          RadioGroup<int>(child: Radio<int>(value: 3)),
        ],
      ),
    );

so I think they can still achieve the same behavior. If this is a very common case, we can consider factor out a pure UI with minimal gesture RadioBase widget that basically just build a selected and unselected state without the value and groupvalue logic, which can be done in a separate pr.

chunhtai · 2025-04-22T19:35:50Z

IMO we should just deprecate the old way - unless is there a way someone could be using Radios that is not compatible with RadioGroup?

If we go with declarative API @justinmc mentioned, I think they should be able to migrate for various use cases. I also can't find a reasonable use case that would prefer the old API. I agree we should deprecate it.

This is in prepare for #167363 The goal for this refactoring is to make sure the new RadioGroup widget can be used for both CupertinoRadio and Radio, and thus the widget has to be in widget layer to avoid duplicating code. Currently CupertinoRadio and Radio shares ToggleableStateMixin in widget layer. but the mixin itself doesn't contains enough data to implement radioGroup. Therefore, I moved more share logic from CupertinoRadio and Radio to widget layer and created a RadioBase widget. ## 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].  [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [Tree Hygiene]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md [test-exempt]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests [Flutter Style Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md [Features we expect every widget to implement]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#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/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md [Data Driven Fixes]: https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md

chunhtai · 2025-05-01T18:11:01Z

close in the favor of #168161

This is in prepare for flutter#167363 The goal for this refactoring is to make sure the new RadioGroup widget can be used for both CupertinoRadio and Radio, and thus the widget has to be in widget layer to avoid duplicating code. Currently CupertinoRadio and Radio shares ToggleableStateMixin in widget layer. but the mixin itself doesn't contains enough data to implement radioGroup. Therefore, I moved more share logic from CupertinoRadio and Radio to widget layer and created a RadioBase widget. ## 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].  [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [Tree Hygiene]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md [test-exempt]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests [Flutter Style Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md [Features we expect every widget to implement]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#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/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md [Data Driven Fixes]: https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md

Piinks reviewed Apr 18, 2025

View reviewed changes

justinmc reviewed Apr 18, 2025

View reviewed changes

chunhtai force-pushed the radio-group branch from b8f485f to 20f206a Compare April 22, 2025 22:47

chunhtai mentioned this pull request Apr 24, 2025

Refactor more logic from radio and cupertino radio #167764

Merged

9 tasks

Adds radio group widget

7ea26c0

chunhtai force-pushed the radio-group branch from 20f206a to 7ea26c0 Compare May 1, 2025 17:09

chunhtai mentioned this pull request May 1, 2025

Adds radio group widget r2 #168161

Merged

9 tasks

chunhtai closed this May 1, 2025

		/// same value type. To use this widget, the [Radio.groupValue] must leave as
		/// null. Otherwise, an assertion error is thrown.

	/// This widget internal builds a [RadioGroupPolicy] to provide an accessible
	/// This widget internally builds a [RadioGroupPolicy] to provide an accessible

Adds radio group widget #167363

Adds radio group widget #167363

Uh oh!

Conversation

chunhtai commented Apr 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pre-launch Checklist

Uh oh!

chunhtai commented Apr 18, 2025

Uh oh!

Piinks left a comment

Choose a reason for hiding this comment

Uh oh!

Piinks Apr 18, 2025

Choose a reason for hiding this comment

Uh oh!

Piinks Apr 18, 2025

Choose a reason for hiding this comment

Uh oh!

Piinks Apr 18, 2025

Choose a reason for hiding this comment

Uh oh!

Piinks Apr 18, 2025

Choose a reason for hiding this comment

Uh oh!

Piinks Apr 18, 2025

Choose a reason for hiding this comment

Uh oh!

Piinks Apr 18, 2025

Choose a reason for hiding this comment

Uh oh!

justinmc left a comment

Choose a reason for hiding this comment

Uh oh!

justinmc Apr 18, 2025

Choose a reason for hiding this comment

Uh oh!

chunhtai commented Apr 22, 2025

Uh oh!

chunhtai commented Apr 22, 2025

Uh oh!

chunhtai commented May 1, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

chunhtai commented Apr 17, 2025 •

edited

Loading