Migrate `IconButton` to Material 3 - Part 1 by QuncCccccc · Pull Request #105176 · flutter/flutter

QuncCccccc · 2022-06-01T22:47:18Z

As part of the migration to Material Design 3, the icon button needs to be updated to the latest design spec:

https://m3.material.io/components/icon-buttons/overview

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

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

This also adds a new style parameter to the IconButton class that allows apps to configure button's ButtonStyle. Similar to the common buttons, IconButton in M3 is a subclass of ButtonStyleButton now.

The second PR will come to enable IconButtons to be toggled.

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.

guidezpl

<3 that there are tests

guidezpl · 2022-06-02T09:11:38Z

examples/api/lib/material/icon_button/icon_button.2.dart

We should use tokens for these. We should also provide the three default styles, so that devs don't have to copy paste and have a nice starting point.

Yeah, we have a similar problem with the variants in the common buttons as well. I didn't realize the variants would need that much configuration.

I really wish we had a simpler way of providing these variants without a lot of extra API baggage to go with them (i.e. either full button subclasses for each variant, or named constructors that have to duplicate all the constructor parameters and use an enum or something to indicate which set of defaults to use in the build method).

That said, I would go with this example for this PR and we can look at cleaning up the variant support in a future PR.

With the current Flutter master channel (9160359), IconButton.styleFrom gives me stadium shapes by default. This is consistent with icon_button.dart#L614, below.

I believe the spec for md.sys.shape.corner.full indicates they should use shape: const CircleBorder() instead.

Hi, the icon button is migrated to be a subclass of ButtonStyleButton now and has a compact VisualDensity on desktop platform, so the buttons don't look like circular. If the app is ran on iOS/Android, the buttons will show a standard VisualDensity and have a circular shape.

If this is the problem that you are facing, adding visualDensity: VisualDensity.standard to IconButton or IconButton.styleFrom() will solve the problem on desktop platform:)

@QuncCccccc Interesting, thanks! I didn't appreciate that property. The spec does have a "high-emphasis filled button for End call" in an example, but it's a bit subtle.

For reference, I see the issue in Flutter web.

Sorry, I assumed you are using a desktop platform. Both desktop and web have a compact visual density as a default value because they have a larger screen, compared to phone devices. So on web, just adding same thing as desktop (visualDensity: VisualDensity.standard) will provide a circular icon button:)

Please let me know if there's any other problems.

guidezpl · 2022-06-02T09:12:13Z

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

Suggested change

/// by configuring the [IconButton] widget's properties.

/// by providing [style].

If defaults are added, mention them here "as a starting point ...standard.copyWith(...)"

guidezpl · 2022-06-02T09:13:58Z

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

My natural question as a developer is, what do I use to specify splash radius? Please add a little more information to these comments.

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

guidezpl · 2022-06-02T09:20:09Z

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

A lot of these have tokens per variant, it may be worth generating 3 separate _TokenDefaultsM3 for each of the styles, instead of my previous recommendation, e.g. md.comp.outlined-icon-button.unselected.outline.color and md.comp.outlined-icon-button.unselected.outline.width.

Agreed, in a follow on API that addresses the variants, we can look at breaking these out.

guidezpl · 2022-06-02T09:24:18Z

In documentation and API, consider supporting 2 types (4 variants total):

Standard icon button
Contained icon button (including filled, filled tonal, and outlined styles)

darrenaustin

Nice work. I have reviewed the main code and it looks good. Just a couple of comments and suggestions below.

For the variants, I suggest just going with the examples you have and we can update this in a future PR after coming up with a better way to handle these.

I haven't had a chance to review the tests yet (thanks for including them 😄). I will look at them with fresh eyes tomorrow.

Until then in addition to the suggestions below, it looks like there are several analyzer warnings that will need to be fixed. Thx.

darrenaustin · 2022-06-03T07:05:48Z

examples/api/lib/material/icon_button/icon_button.2.dart

Yeah, we have a similar problem with the variants in the common buttons as well. I didn't realize the variants would need that much configuration.

I really wish we had a simpler way of providing these variants without a lot of extra API baggage to go with them (i.e. either full button subclasses for each variant, or named constructors that have to duplicate all the constructor parameters and use an enum or something to indicate which set of defaults to use in the build method).

That said, I would go with this example for this PR and we can look at cleaning up the variant support in a future PR.

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

darrenaustin · 2022-06-03T07:11:38Z

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

Agreed, in a follow on API that addresses the variants, we can look at breaking these out.

darrenaustin

LGTM. Nice work.

Looks like there are still some analyzer failures that will need to be fixed before we can merge this.

darrenaustin · 2022-06-07T07:59:12Z

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

Do the button style buttons not require a Material widget as well?

No:) The ButtonStyleButton has its Material widget inside, but the original IconButton doesn't have one.

…lined types

…105176)

* Added standard IconButton for M3 with new ButtonStyle field * Added IconButton examples for standard, filled, filled_tonal, and outlined types Co-authored-by: Qun Cheng <[email protected]>

…105176)

kirolous-nashaat · 2022-10-14T22:05:20Z

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

+      final Size? minSize = constraints == null
+          ? null
+          : Size(constraints!.minWidth, constraints!.minHeight);
+      final Size? maxSize = constraints == null
+          ? null
+          : Size(constraints!.maxWidth, constraints!.maxHeight);


Hello @QuncCccccc , quick question
Why are you calculating new size, shouldn't we be using adjustedConstraints instead ?
This results in non-square icon buttons which depends on the icon itself causing weird behavior.

QuncCccccc force-pushed the migrate_icon_button_style branch 3 times, most recently from a2ffa24 to 878d83d Compare June 2, 2022 05:28

QuncCccccc marked this pull request as ready for review June 2, 2022 05:30

QuncCccccc requested review from darrenaustin and guidezpl June 2, 2022 05:30

guidezpl reviewed Jun 2, 2022

View reviewed changes

QuncCccccc changed the title ~~Migrate common buttons to Material 3 - Part 1~~ Migrate IconButton to Material 3 - Part 1 Jun 2, 2022

QuncCccccc force-pushed the migrate_icon_button_style branch from ca82cb2 to ed78ae3 Compare June 2, 2022 22:44

darrenaustin reviewed Jun 3, 2022

View reviewed changes

QuncCccccc force-pushed the migrate_icon_button_style branch from 8c40aa5 to a258a49 Compare June 3, 2022 19:43

QuncCccccc requested review from darrenaustin and guidezpl June 6, 2022 17:44

QuncCccccc force-pushed the migrate_icon_button_style branch from a258a49 to c1447d4 Compare June 6, 2022 17:52

darrenaustin approved these changes Jun 7, 2022

View reviewed changes

QuncCccccc added 7 commits June 7, 2022 10:17

Added standard IconButton for M3 with new ButtonStyle field

d252e7c

Added IconButton examples for standart, filled, filled_tonal, and out…

90cfcee

…lined types

Changed old tests and added new tests

aad71df

Fixed comments about naming and documentation

f252830

Fixed failed analyze

e4f8186

Addressed comments and test failures

113c213

Fixed analyzer failure

7f6e9d5

QuncCccccc force-pushed the migrate_icon_button_style branch from b774d7a to 7f6e9d5 Compare June 7, 2022 17:20

QuncCccccc merged commit 66a84d1 into flutter:master Jun 7, 2022

engine-flutter-autoroll mentioned this pull request Jun 8, 2022

Roll Flutter from be0c1bd0f0ce to 4ec296513cb2 (24 revisions) flutter/plugins#5935

Merged

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

66a84d1 Migrate IconButton to Material 3 - Part 1 (flutter/flutter#…

81803c3

…105176)

engine-flutter-autoroll mentioned this pull request Jun 8, 2022

Roll Flutter from 5ee59fa9ec15 to 9202e69febca (20 revisions) flutter/packages#2224

Merged

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

66a84d1 Migrate IconButton to Material 3 - Part 1 (flutter/flutter#…

2d48150

…105176)

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

66a84d1 Migrate IconButton to Material 3 - Part 1 (flutter/flutter#…

d6f7c79

…105176)

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

66a84d1 Migrate IconButton to Material 3 - Part 1 (flutter/flutter#…

df04c36

…105176)

kirolous-nashaat reviewed Oct 14, 2022

View reviewed changes

	/// by configuring the [IconButton] widget's properties.
	/// by providing [style].

Conversation

QuncCccccc commented Jun 1, 2022 • edited by guidezpl Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pre-launch Checklist

Uh oh!

guidezpl left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

guidezpl Jun 2, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

guidezpl commented Jun 2, 2022

Uh oh!

darrenaustin left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

darrenaustin left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

QuncCccccc commented Jun 1, 2022 •

edited by guidezpl

Loading

guidezpl Jun 2, 2022 •

edited

Loading