Migrated `Switch` to Material 3 #110095

QuncCccccc · 2022-08-23T16:01:13Z

Part of: #91605

Updated the Switch widget with support for Material Design 3.

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

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

Fixes: #103536

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.

mavyfaby · 2022-08-26T11:10:53Z

Any updates?

darrenaustin

@QuncCccccc sorry this took so long to review. There is a lot here!

Looks great. Nice work. I have a question about the defaults vs theme data below.

In a future PR we should add an example or two showing how to use the new thumb image and icons, as well as update the demo app.

darrenaustin · 2022-08-26T18:15:44Z

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

+  ///   onChanged: (_) => true,
+  ///   thumbImage: MaterialStateProperty.resolveWith<ImageProvider>((Set<MaterialState> states) {
+  ///     if (states.contains(MaterialState.disabled)) {
+  ///       return MemoryImage(Uint8List.fromList(<int>[1, 2]));


For this example, perhaps just have disabledImage and normalImage variables for the images so that we don't have to complicate the example with the details of MemoryImage.

Small question: do you really want thumbImage? Can't I use Icon(Icons.check) to do the check? And should there be only a single thumbImage? In the Material specs they use x for unselected, check for selected.

darrenaustin · 2022-08-26T18:24:45Z

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

@@ -1078,3 +1322,288 @@ class _SwitchPainter extends ToggleablePainter {
    super.dispose();
  }
 }
+
+class _SwitchDefaults {


So is the reason for this class because we have a bunch of values that need to change between M2 and M3, but they aren't in the SwitchThemeData? If so, perhaps we should just add them to the ThemeData so that apps can customize them if they want. That would be consistent with the way we handle other components. Although there is a lot here, that maybe we don't want to bloat the ThemeData with.

If we don't want to make them part of the ThemeData, then it might be cleaner to just have a _SwitchConfig interface and have a hand coded _SwitchConfigM2 and the generated _SwitchConfigM3 that implement it. That will make it easier to ditch the M2 implementation when we switch over to M3 completely.

Thanks a lot for the suggestions! Just created a separate config class for both M2 and M3. Please let me know if there're any problems:)

packages/flutter/test/material/switch_test.dart

HansMuller

I haven't finished reviewing this yet

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

HansMuller · 2022-08-26T20:57:50Z

dev/tools/gen_defaults/lib/switch_template.dart

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


Should this be Colors.transparent? How do we expect overlayColor=null to be interpreted in this case?

CC @darrenaustin - Maybe this is another case where we want null to mean "no overlay" instead of "no preference".

When the switch is not in any of the states(hovered, focused, and pressed), should we just give a null value for the state layer?

HansMuller · 2022-08-26T21:00:06Z

dev/tools/gen_defaults/lib/switch_template.dart

+  BuildContext context;
+  final ColorScheme _colors;
+
+  static const double iconSize = ${tokens['md.comp.switch.unselected.icon.size']};


Good to see all of these configuration parameters being derived from the token db.

HansMuller

This looks great (good to see all of the tests). I realize that there's still feedback from Darren that needs to be resolved.

HansMuller · 2022-08-29T16:19:21Z

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

+  /// The optional icon on the thumb of this switch when the switch is on.
+  ///
+  /// This property can be null.
+  final IconData? activeIcon;


There are more Switch color and thumb image configuration parameters than we need, given MaterialStateProperty. In a future PR and design doc, I think we should either migrate developers to a new switch widget, one that only has MaterialStateProperty parameters, or a new version of Switch that deprecates all of the excess baggage.

HansMuller · 2022-08-29T17:34:37Z

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

@@ -441,11 +498,11 @@ class Switch extends StatelessWidget {
        height: size.height,
        alignment: Alignment.center,
        child: CupertinoSwitch(


Similar to my comment about Switch's icon and color properties: in the future, perhaps we can avoid delegating to a Cupertino library widget, and just configure the Material Switch widget instead.

…ethod

HansMuller · 2022-09-07T00:04:36Z

@bernaferrari (per #110095 (comment)) I agree that just using "thumb" instead of "thumbImage" would make the API more concise, using thumbImage has two advantages:

It doesn't completely define the appearance of the thumb, just an image that that's drawn on the thumb
It makes its relationship with the existing cloud of fooThumbImage properties obvious.

bernaferrari · 2022-09-07T01:33:47Z

Yeah, but for me the "image" part is kind of confusing as it is not really an image when it is something else. An icon is supposedly different from an image. So I think something like thumbForeground could make more sense so people know anything can go there.

darrenaustin

Looks good. I a have a couple of small comments and a question about thumbImage below.

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

darrenaustin · 2022-09-07T18:52:23Z

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

@@ -1217,7 +1217,7 @@ class ThemeData with Diagnosticable {
  ///   * Typography: `typography` (see table above)
  ///
  /// ### Components
-  ///   * Common buttons: [ElevatedButton], [FilledButton], [OutlinedButton], [TextButton]
+  ///   * Common buttons: [ElevatedButton], [FilledButton], [OutlinedButton], [TextButton], [IconButton]


Ah we missed this one. Nice catch.

darrenaustin · 2022-09-07T19:04:41Z

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

+    if (thumbWidget is Image) {
+      effectiveActiveThumbImage = (widget.thumbImage?.resolve(activeStates) as Image?)?.image;
+      effectiveInactiveThumbImage = (widget.thumbImage?.resolve(inactiveStates) as Image?)?.image;
+    }
+    final Widget? thumbThemeWidget = switchTheme.thumbImage?.resolve(states);
+    effectiveActiveThumbImage ??= _widgetThumbImage.resolve(activeStates);
+    effectiveInactiveThumbImage ??= _widgetThumbImage.resolve(inactiveStates);
+    if (thumbThemeWidget is Image) {
+      effectiveActiveThumbImage ??= (switchTheme.thumbImage?.resolve(activeStates) as Image?)?.image;
+      effectiveInactiveThumbImage ??= (switchTheme.thumbImage?.resolve(inactiveStates) as Image?)?.image;
+    }
+
+    Icon? effectiveActiveIcon;
+    Icon? effectiveInactiveIcon;
+    if (theme.useMaterial3) {
+      if (thumbWidget is Icon) {
+        effectiveActiveIcon = widget.thumbImage?.resolve(activeStates) as Icon?;
+        effectiveInactiveIcon = widget.thumbImage?.resolve(inactiveStates) as Icon?;
+      }
+      if (thumbThemeWidget is Icon) {
+        effectiveActiveIcon ??= switchTheme.thumbImage?.resolve(activeStates) as Icon?;
+        effectiveInactiveIcon ??= switchTheme.thumbImage?.resolve(inactiveStates) as Icon?;
+      }
+    }


How is thumbImage used if it is not an Icon or Image? What if it was just a label or a container with a shape and color? Will it be rendered on the thumb of the switch?

Or do we only support image and icon widgets for thumbImage? If so we would at least need to document that restriction.

Yeah, the thumbImage currently only supports Image and Icon. Investigating how we can paint a widget directly. Thanks for the suggestion!

This is the challenge, because it uses a BoxDecoration and pre-renders/caches it. I don't know how to improve that.
_cachedThumbPainter = _createDefaultThumbDecoration(thumbColor, thumbImage, thumbErrorListener).createBoxPainter(_handleDecorationChanged);

Yes, this is tricky. But I agree that the restriction that only allows 2 types here is a little odd. To support the optional icon feature, I changed the thumbImage to the thumbIcon, and just used the type of MaterialStateProperty<Icon?>? instead of MaterialStateProperty<Widget?>?. Eventually, we will refactor this part of the code and allow a widget painting on the thumb of Switch.

…material3

HansMuller

LGTM

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

darrenaustin

Nice work.

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 23, 2022

QuncCccccc requested review from darrenaustin and HansMuller August 23, 2022 16:29

QuncCccccc force-pushed the update_switch_to_material3 branch 2 times, most recently from a128d1e to 6be39fe Compare August 24, 2022 06:38

QuncCccccc marked this pull request as ready for review August 24, 2022 16:45

QuncCccccc requested review from darrenaustin and HansMuller and removed request for darrenaustin and HansMuller August 24, 2022 20:32

darrenaustin reviewed Aug 26, 2022

View reviewed changes

HansMuller reviewed Aug 26, 2022

View reviewed changes

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

HansMuller reviewed Aug 26, 2022

View reviewed changes

HansMuller approved these changes Aug 29, 2022

View reviewed changes

Qun Cheng added 11 commits August 29, 2022 14:59

Migrated the Switch to Material3

d32a161

Removed indent

4bd1828

Added test

b616e63

Added tests for thumbImage

c675e2a

Fixed failing test

6967db6

Removed trailing space

9e080c5

Changed the test number 1.0 to 1

38beefb

Changed the tested double number

f08cb78

Removed extra comments

5ab248c

Added switch examples for the new properties and fixed partial comments

2effbd8

Reorganized the config data for M2 and M3

f720896

QuncCccccc force-pushed the update_switch_to_material3 branch from 6d40c0b to f720896 Compare August 30, 2022 18:32

Corrected the order of pressed, hovered, and focused in the resolve m…

f96f37e

…ethod

Removed unused import

7b42c6f

Fixed failing test

f978ba5

darrenaustin reviewed Sep 7, 2022

View reviewed changes

Qun Cheng added 3 commits September 7, 2022 16:54

Changed doc

ca076c4

Merge remote-tracking branch 'upstream/master' into update_switch_to_…

0236248

…material3

Changed thumbImage to thumbIcon which only allows Icon type

48feade

QuncCccccc requested review from darrenaustin and HansMuller September 8, 2022 23:02

Fixed failing test

6932d03

HansMuller reviewed Sep 8, 2022

View reviewed changes

packages/flutter/lib/src/material/switch.dart Show resolved Hide resolved

Removed all the extra indentation

3398c23

darrenaustin approved these changes Sep 9, 2022

View reviewed changes

QuncCccccc merged commit 75fac6a into flutter:master Sep 9, 2022
122 checks passed

This was referenced Sep 10, 2022

Roll Flutter from ad946f8478bf to 4930444f4afe (31 revisions) flutter/packages#2588

Merged

Roll Flutter from 4cd734ae3a1c to 4930444f4afe (28 revisions) flutter/plugins#6386

Closed

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

75fac6ae4 Migrated Switch to Material 3 (flutter/flutter#110095)

1ea7217

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

75fac6a Migrated Switch to Material 3 (flutter/flutter#110095)

c1b7943

engine-flutter-autoroll mentioned this pull request Sep 10, 2022

Roll Flutter from 4cd734ae3a1c to 4930444f4afe (28 revisions) flutter/plugins#6387

Closed

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

75fac6a Migrated Switch to Material 3 (flutter/flutter#110095)

1361a5e

engine-flutter-autoroll mentioned this pull request Sep 11, 2022

Roll Flutter from 4cd734ae3a1c to 4930444f4afe (28 revisions) flutter/plugins#6388

Closed

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

75fac6a Migrated Switch to Material 3 (flutter/flutter#110095)

7abda94

engine-flutter-autoroll mentioned this pull request Sep 12, 2022

Roll Flutter from 4cd734ae3a1c to 7ad47233a9a3 (29 revisions) flutter/plugins#6410

Closed

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

75fac6a Migrated Switch to Material 3 (flutter/flutter#110095)

11ef2c3

engine-flutter-autoroll mentioned this pull request Sep 12, 2022

Roll Flutter from 4cd734ae3a1c to 6b56654890b0 (30 revisions) flutter/plugins#6412

Closed

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

75fac6a Migrated Switch to Material 3 (flutter/flutter#110095)

ec70511

engine-flutter-autoroll mentioned this pull request Sep 12, 2022

Roll Flutter from 4cd734ae3a1c to 81fd386a7fdb (32 revisions) flutter/plugins#6413

Closed

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

75fac6a Migrated Switch to Material 3 (flutter/flutter#110095)

b572c0c

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

75fac6a Migrated Switch to Material 3 (flutter/flutter#110095)

8becdbb

Migrated `Switch` to Material 3 #110095

Migrated `Switch` to Material 3 #110095

QuncCccccc commented Aug 23, 2022

mavyfaby commented Aug 26, 2022

darrenaustin left a comment

darrenaustin Aug 26, 2022 •

edited

bernaferrari Aug 30, 2022 •

edited

darrenaustin Aug 26, 2022

QuncCccccc Aug 30, 2022

HansMuller left a comment

HansMuller Aug 26, 2022

QuncCccccc Aug 30, 2022

HansMuller Aug 26, 2022

HansMuller left a comment

HansMuller Aug 29, 2022

HansMuller Aug 29, 2022

HansMuller commented Sep 7, 2022

bernaferrari commented Sep 7, 2022 •

edited

darrenaustin left a comment

darrenaustin Sep 7, 2022

darrenaustin Sep 7, 2022

QuncCccccc Sep 8, 2022

bernaferrari Sep 8, 2022

QuncCccccc Sep 8, 2022

bernaferrari Sep 9, 2022

HansMuller left a comment

darrenaustin left a comment

Migrated Switch to Material 3 #110095

Migrated Switch to Material 3 #110095

Conversation

QuncCccccc commented Aug 23, 2022

Pre-launch Checklist

mavyfaby commented Aug 26, 2022

darrenaustin left a comment

Choose a reason for hiding this comment

darrenaustin Aug 26, 2022 • edited

Choose a reason for hiding this comment

bernaferrari Aug 30, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

HansMuller left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

HansMuller left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

HansMuller commented Sep 7, 2022

bernaferrari commented Sep 7, 2022 • edited

darrenaustin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

HansMuller left a comment

Choose a reason for hiding this comment

darrenaustin left a comment

Choose a reason for hiding this comment

Migrated `Switch` to Material 3 #110095

Migrated `Switch` to Material 3 #110095

darrenaustin Aug 26, 2022 •

edited

bernaferrari Aug 30, 2022 •

edited

bernaferrari commented Sep 7, 2022 •

edited