Migrated `Checkbox` to Material 3 Colors #110537

QuncCccccc · 2022-08-29T21:35:54Z

Updated checkbox colors so they use the material 3 palette.

In order to use the Checkbox with the new Material 3 defaults, turn on the useMaterial3 flag in the ThemeData:

  return MaterialApp(
    theme: ThemeData(useMaterial3: true),
    // ...
  );

#104446

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.

HansMuller

It seems possible that the implementation of this widget could be made substantially simpler in a future PR. Perhaps the code that computes "effective" colors by adding a state to the widgets actual state, could be deferred. So that the painter only computed those colors when it needed them. The division of labor between the Toggleable classes and this class seems like more overhead than should be needed.

HansMuller · 2022-08-29T22:41:37Z

dev/tools/gen_defaults/lib/checkbox_template.dart

+      if (states.contains(MaterialState.pressed)) {
+        return ${componentColor('md.comp.checkbox.unselected.pressed.state-layer')};
+      }
+      return null;


CC @darrenaustin
Should this be Colors.transparent?

HansMuller · 2022-08-29T22:42:30Z

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

-      ?? checkboxTheme.fillColor?.resolve(inactiveStates)
-      ?? _defaultFillColor.resolve(inactiveStates);
+    final Color? activeColor = widget.fillColor?.resolve(activeStates)
+        ?? _widgetFillColor.resolve(activeStates)


two-space indent here and below

HansMuller · 2022-08-29T22:58:35Z

dev/tools/gen_defaults/lib/checkbox_template.dart

+        if (states.contains(MaterialState.hovered)) {
+          return ${componentColor('md.comp.checkbox.selected.hover.container')};
+        }
+        if (states.contains(MaterialState.pressed)) {


Should the pressed test come first (here and elsewhere)? When pressed becomes true, hovered will also always be true, and so we'll miss this if statement.

Fixed. Thanks for catching this!

guidezpl

A couple of TODOs

CheckboxThemeData.side needs to support MaterialStateProperty<Side> instead of Side
Error states

Are you planning on addressing these in this PR or a future one?

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

guidezpl

One more thing, the useMaterial3 documentation should be updated

QuncCccccc · 2022-08-30T19:39:32Z

A couple of TODOs

CheckboxThemeData.side needs to support MaterialStateProperty<Side> instead of Side

Error states

Are you planning on addressing these in this PR or a future one?

I will create a separate PR for the TODOs:) Thanks!

…rialStateProperty method

HansMuller

LGTM

HansMuller · 2022-08-31T16:22:05Z

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

+        return _colors.onSurface.withOpacity(0.38);
+      }
+      if (states.contains(MaterialState.selected)) {
+        if (states.contains(MaterialState.pressed)) {


TODO (future PR): update the code generator to collapse code like this.

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

guidezpl

Edited the description to not close the issue, pending the remaining work.

…10537)

)

Qun Cheng added 3 commits August 29, 2022 13:36

Updated Checkbox component

0145371

Removed extra commented code

c3e7214

Reformatted code

b568ee1

flutter-dashboard bot added f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels. team Infra upgrades, team productivity, code health, technical debt. See also team: labels. labels Aug 29, 2022

Removed trailing space

fa30f30

QuncCccccc marked this pull request as ready for review August 29, 2022 22:33

QuncCccccc requested review from darrenaustin, guidezpl and HansMuller August 29, 2022 22:33

HansMuller reviewed Aug 29, 2022

View reviewed changes

guidezpl reviewed Aug 30, 2022

View reviewed changes

packages/flutter/lib/src/material/checkbox.dart Outdated Show resolved Hide resolved

guidezpl reviewed Aug 30, 2022

View reviewed changes

Qun Cheng added 2 commits August 30, 2022 13:20

addressed comments

c665acb

Corrected the order of pressed, hovered, and focused when handle Mate…

c2e977a

…rialStateProperty method

HansMuller approved these changes Aug 31, 2022

View reviewed changes

guidezpl approved these changes Aug 31, 2022

View reviewed changes

QuncCccccc changed the title ~~Migrated Checkbox to Material 3~~ Migrated Checkbox to Material 3 Colors Aug 31, 2022

Fixed commnets

2db2c9a

QuncCccccc added the autosubmit Merge PR when tree becomes green via auto submit App label Aug 31, 2022

auto-submit bot merged commit a490a6a into flutter:master Aug 31, 2022

This was referenced Sep 1, 2022

Roll Flutter from f66a09791920 to 759d0e1b20c1 (56 revisions) flutter/packages#2541

Merged

Roll Flutter from 069f5042973d to 759d0e1b20c1 (16 revisions) flutter/plugins#6349

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 1, 2022

a490a6a11 Migrated Checkbox to Material 3 Colors (flutter/flutter#1…

53c6c14

…10537)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Sep 1, 2022

a490a6a Migrated Checkbox to Material 3 Colors (flutter/flutter#110537

b8fa342

)

darrenaustin mentioned this pull request Sep 2, 2022

☂️ Bring Material 3 to Flutter #91605

Closed

70 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Migrated `Checkbox` to Material 3 Colors #110537

Migrated `Checkbox` to Material 3 Colors #110537

QuncCccccc commented Aug 29, 2022 •

edited by guidezpl

HansMuller left a comment

HansMuller Aug 29, 2022

HansMuller Aug 29, 2022

QuncCccccc Aug 30, 2022

HansMuller Aug 29, 2022

QuncCccccc Aug 30, 2022

guidezpl left a comment

guidezpl left a comment

QuncCccccc commented Aug 30, 2022

HansMuller left a comment

HansMuller Aug 31, 2022

guidezpl left a comment

Migrated Checkbox to Material 3 Colors #110537

Migrated Checkbox to Material 3 Colors #110537

Conversation

QuncCccccc commented Aug 29, 2022 • edited by guidezpl

Pre-launch Checklist

HansMuller left a comment

Choose a reason for hiding this comment

HansMuller Aug 29, 2022

Choose a reason for hiding this comment

HansMuller Aug 29, 2022

Choose a reason for hiding this comment

QuncCccccc Aug 30, 2022

Choose a reason for hiding this comment

HansMuller Aug 29, 2022

Choose a reason for hiding this comment

QuncCccccc Aug 30, 2022

Choose a reason for hiding this comment

guidezpl left a comment

Choose a reason for hiding this comment

guidezpl left a comment

Choose a reason for hiding this comment

QuncCccccc commented Aug 30, 2022

HansMuller left a comment

Choose a reason for hiding this comment

HansMuller Aug 31, 2022

Choose a reason for hiding this comment

guidezpl left a comment

Choose a reason for hiding this comment

Migrated `Checkbox` to Material 3 Colors #110537

Migrated `Checkbox` to Material 3 Colors #110537

QuncCccccc commented Aug 29, 2022 •

edited by guidezpl