Skip to content

Add backgroundColor to Radio #169415

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conversation

ValentinVignal
Copy link
Contributor

Part of #168787

Adds backgroundColor property to Radio

This code sample is now possible:

Example
// Copyright 2014 The Flutter Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

import 'package:flutter/material.dart';

void main() {
  runApp(const MyWidget());
}

class MyWidget extends StatefulWidget {
  const MyWidget({super.key});

  @override
  State<MyWidget> createState() => _MyWidgetState();
}

class _MyWidgetState extends State<MyWidget> {
  bool? _value;

  @override
  Widget build(BuildContext context) {
    return MaterialApp(
      home: Scaffold(
        body: Column(
          children: <Widget>[
            Radio<bool>(
              value: true,
              groupValue: _value,
              toggleable: true,
              onChanged: (bool? value) {
                setState(() {
                  // Toggle the value when the radio button is pressed
                  _value = value;
                });
              },
              activeColor: Colors.red,
              fillColor: MaterialStateProperty.resolveWith<Color>((Set<MaterialState> states) {
                if (states.contains(MaterialState.selected)) {
                  return Colors.green;
                }
                return Colors.blue; // Default color when not selected
              }),
              backgroundColor: MaterialStateProperty.resolveWith<Color>((
                Set<MaterialState> states,
              ) {
                if (states.contains(MaterialState.selected)) {
                  return Colors.orange.withOpacity(0.5);
                }
                return Colors.purple.withOpacity(0.5); // Default background color when not selected
              }),
            ),
            Radio<bool>(
              value: false,
              groupValue: _value,
              toggleable: true,
              onChanged: (bool? value) {
                setState(() {
                  // Toggle the value when the radio button is pressed
                  _value = value;
                });
              },
            ),
          ],
        ),
      ),
    );
  }
}
Screen.Recording.2025-05-24.at.10.18.04.PM.mov

Pre-launch Checklist

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

@github-actions github-actions bot added framework flutter/packages/flutter repository. See also f: labels. f: material design flutter/packages/flutter/material repository. labels May 24, 2025
@ValentinVignal
Copy link
Contributor Author

ValentinVignal commented May 24, 2025

This PR is to kickstart the fixes for #168787

I would like to do it iteratively.
This PR only includes the Radio.backgroundColor property. In future PRs I can:

  • Add backgroundColor to RadioThemeData
  • Add backgroundColor to CupertinoRadio
  • Add radioBackgroundColor to RadioListTile
  • Add the other changes for Add ways to customise Radio #168787
    if those are acceptable/wanted.

Comment on lines +488 to +621
final Color activeBackgroundColor =
widget.backgroundColor?.resolve(activeStates) ?? Colors.transparent;
final Color inactiveBackgroundColor =
widget.backgroundColor?.resolve(inactiveStates) ?? Colors.transparent;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure how we want to handle those transparent colors? The other properties are using _RadioDefaultsM3 or _RadioDefaultsM2 as a fallbacks.

What is the best thing to do here?

Copy link
Contributor

Choose a reason for hiding this comment

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

@QuncCccccc Is this something that could/should be in the Material token database? Or is this ok as-is?

Copy link
Contributor

Choose a reason for hiding this comment

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

At some point the property will be in RadioTheme right? We should default to the property in RadioTheme instead

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree that it should ultimately be included in the theme, @chunhtai.

However, as I mentioned in this comment #169415 (comment), my intention was to approach this iteratively and address the theme in a future PR. This way, I can keep this PR (and future ones) small and focused.

Would that be acceptable to you? If not, could you please specify the minimal set of changes required for this PR?

Copy link
Contributor

Choose a reason for hiding this comment

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

sgtm as iterative change. consider add a todo here with the link to the issue for adding themedata so we don't miss this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Makes sense, I added a TODO in Add TODO

Copy link
Contributor

@justinmc justinmc left a comment

Choose a reason for hiding this comment

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

FYI @chunhtai, who recently did some Radio refactoring in #168161.

Overall looks good to me.

Comment on lines +488 to +621
final Color activeBackgroundColor =
widget.backgroundColor?.resolve(activeStates) ?? Colors.transparent;
final Color inactiveBackgroundColor =
widget.backgroundColor?.resolve(inactiveStates) ?? Colors.transparent;
Copy link
Contributor

Choose a reason for hiding this comment

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

@QuncCccccc Is this something that could/should be in the Material token database? Or is this ok as-is?

@ValentinVignal ValentinVignal force-pushed the flutter/add-background-color-to-radio branch from d9e5ffd to ab03c42 Compare June 4, 2025 12:23
@@ -106,6 +106,7 @@ class Radio<T> extends StatefulWidget {
this.autofocus = false,
this.enabled,
this.groupRegistry,
this.backgroundColor,
Copy link
Contributor

Choose a reason for hiding this comment

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

We should also add this to RadioListTile

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Similar to #169415 (comment), I was considering adding it with another PR. Would that be fine with you?

Copy link
Contributor

Choose a reason for hiding this comment

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

yes

/// * [WidgetState.disabled].
///
/// If null, then it is transparent in all states.
final MaterialStateProperty<Color?>? backgroundColor;
Copy link
Contributor

Choose a reason for hiding this comment

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

MaterialStateProperty is deprecated, use WidgetStateProperty

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I used WidgetStateProperty in Use WidgetStateProperty and RadioGroup

Comment on lines +488 to +621
final Color activeBackgroundColor =
widget.backgroundColor?.resolve(activeStates) ?? Colors.transparent;
final Color inactiveBackgroundColor =
widget.backgroundColor?.resolve(inactiveStates) ?? Colors.transparent;
Copy link
Contributor

Choose a reason for hiding this comment

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

At some point the property will be in RadioTheme right? We should default to the property in RadioTheme instead

key: radioKey,
value: 0,
fillColor: backgroundColor,
onChanged:
Copy link
Contributor

Choose a reason for hiding this comment

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

Use radioGroup instead

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ValentinVignal ValentinVignal force-pushed the flutter/add-background-color-to-radio branch from ab03c42 to 608a2e6 Compare June 5, 2025 09:54
@ValentinVignal ValentinVignal requested a review from chunhtai June 5, 2025 09:55
Copy link
Contributor

@chunhtai chunhtai left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -106,6 +106,7 @@ class Radio<T> extends StatefulWidget {
this.autofocus = false,
this.enabled,
this.groupRegistry,
this.backgroundColor,
Copy link
Contributor

Choose a reason for hiding this comment

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

yes

Comment on lines +488 to +621
final Color activeBackgroundColor =
widget.backgroundColor?.resolve(activeStates) ?? Colors.transparent;
final Color inactiveBackgroundColor =
widget.backgroundColor?.resolve(inactiveStates) ?? Colors.transparent;
Copy link
Contributor

Choose a reason for hiding this comment

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

sgtm as iterative change. consider add a todo here with the link to the issue for adding themedata so we don't miss this

@ValentinVignal ValentinVignal force-pushed the flutter/add-background-color-to-radio branch from 608a2e6 to 343aca3 Compare June 6, 2025 01:22
@ValentinVignal ValentinVignal requested a review from justinmc June 6, 2025 01:22
@ValentinVignal ValentinVignal added the autosubmit Merge PR when tree becomes green via auto submit App label Jun 10, 2025
@auto-submit auto-submit bot added this pull request to the merge queue Jun 10, 2025
Merged via the queue into flutter:master with commit b086fe7 Jun 10, 2025
75 of 76 checks passed
@flutter-dashboard flutter-dashboard bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Jun 10, 2025
@ValentinVignal ValentinVignal mentioned this pull request Jun 10, 2025
9 tasks
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants