[material_ui] Migrate M3 buttons template to use new gen_defaults - #12744
[material_ui] Migrate M3 buttons template to use new gen_defaults#12744QuncCccccc wants to merge 1 commit into
Conversation
c982306 to
c3c6211
Compare
bc042ae to
e5653f7
Compare
e5653f7 to
a5b9d6d
Compare
There was a problem hiding this comment.
Code Review
This pull request migrates Material 3 button defaults to be generated from tokens via a new ButtonTemplateM3 template, removing hardcoded default classes from the button implementation files and generating them as part files instead. The review feedback suggests modernizing the generated code by replacing deprecated MaterialStatePropertyAll references with WidgetStatePropertyAll in both the template and its associated tests.
| String get _backgroundColor { | ||
| final TokenColorRole? containerColor = _containerColor; | ||
| if (containerColor == null) { | ||
| return 'const MaterialStatePropertyAll<Color>(Colors.transparent)'; | ||
| } |
There was a problem hiding this comment.
Since this is a new template migration, we should use the modern WidgetStatePropertyAll instead of the legacy/deprecated MaterialStatePropertyAll to align with Flutter's ongoing migration from MaterialState to WidgetState.
| String get _backgroundColor { | |
| final TokenColorRole? containerColor = _containerColor; | |
| if (containerColor == null) { | |
| return 'const MaterialStatePropertyAll<Color>(Colors.transparent)'; | |
| } | |
| String get _backgroundColor { | |
| final TokenColorRole? containerColor = _containerColor; | |
| if (containerColor == null) { | |
| return 'const WidgetStatePropertyAll<Color>(Colors.transparent)'; | |
| } |
| String get _shadowColor { | ||
| return switch (_variant) { | ||
| _ButtonVariant.elevated => | ||
| 'MaterialStatePropertyAll<Color>(${color(TokenButtonElevated.containerShadowColor, '_colors')})', | ||
| _ButtonVariant.filled => | ||
| 'MaterialStatePropertyAll<Color>(${color(TokenButtonFilled.containerShadowColor, '_colors')})', | ||
| _ButtonVariant.filledTonal => | ||
| 'MaterialStatePropertyAll<Color>(${color(TokenButtonTonal.containerShadowColor, '_colors')})', | ||
| _ButtonVariant.outlined || | ||
| _ButtonVariant.text => 'const MaterialStatePropertyAll<Color>(Colors.transparent)', | ||
| }; | ||
| } |
There was a problem hiding this comment.
Use WidgetStatePropertyAll instead of MaterialStatePropertyAll for consistency and modernization.
| String get _shadowColor { | |
| return switch (_variant) { | |
| _ButtonVariant.elevated => | |
| 'MaterialStatePropertyAll<Color>(${color(TokenButtonElevated.containerShadowColor, '_colors')})', | |
| _ButtonVariant.filled => | |
| 'MaterialStatePropertyAll<Color>(${color(TokenButtonFilled.containerShadowColor, '_colors')})', | |
| _ButtonVariant.filledTonal => | |
| 'MaterialStatePropertyAll<Color>(${color(TokenButtonTonal.containerShadowColor, '_colors')})', | |
| _ButtonVariant.outlined || | |
| _ButtonVariant.text => 'const MaterialStatePropertyAll<Color>(Colors.transparent)', | |
| }; | |
| } | |
| String get _shadowColor { | |
| return switch (_variant) { | |
| _ButtonVariant.elevated => | |
| 'WidgetStatePropertyAll<Color>(${color(TokenButtonElevated.containerShadowColor, '_colors')})', | |
| _ButtonVariant.filled => | |
| 'WidgetStatePropertyAll<Color>(${color(TokenButtonFilled.containerShadowColor, '_colors')})', | |
| _ButtonVariant.filledTonal => | |
| 'WidgetStatePropertyAll<Color>(${color(TokenButtonTonal.containerShadowColor, '_colors')})', | |
| _ButtonVariant.outlined || | |
| _ButtonVariant.text => 'const WidgetStatePropertyAll<Color>(Colors.transparent)', | |
| }; | |
| } |
| String get _elevation { | ||
| if (_variant == _ButtonVariant.outlined || _variant == _ButtonVariant.text) { | ||
| return 'const MaterialStatePropertyAll<double>(0.0)'; | ||
| } |
There was a problem hiding this comment.
Use WidgetStatePropertyAll instead of MaterialStatePropertyAll for the outlined and text button elevation defaults.
| String get _elevation { | |
| if (_variant == _ButtonVariant.outlined || _variant == _ButtonVariant.text) { | |
| return 'const MaterialStatePropertyAll<double>(0.0)'; | |
| } | |
| String get _elevation { | |
| if (_variant == _ButtonVariant.outlined || _variant == _ButtonVariant.text) { | |
| return 'const WidgetStatePropertyAll<double>(0.0)'; | |
| } |
| @override | ||
| WidgetStateProperty<TextStyle?> get textStyle => | ||
| MaterialStatePropertyAll<TextStyle?>($_buttonTextStyle); |
There was a problem hiding this comment.
Use WidgetStatePropertyAll to match the WidgetStateProperty return type.
| @override | |
| WidgetStateProperty<TextStyle?> get textStyle => | |
| MaterialStatePropertyAll<TextStyle?>($_buttonTextStyle); | |
| @override | |
| WidgetStateProperty<TextStyle?> get textStyle => | |
| WidgetStatePropertyAll<TextStyle?>($_buttonTextStyle); |
| @override | ||
| WidgetStateProperty<Color>? get surfaceTintColor => | ||
| const MaterialStatePropertyAll<Color>(Colors.transparent); |
There was a problem hiding this comment.
Use WidgetStatePropertyAll instead of MaterialStatePropertyAll for surfaceTintColor.
| @override | |
| WidgetStateProperty<Color>? get surfaceTintColor => | |
| const MaterialStatePropertyAll<Color>(Colors.transparent); | |
| @override | |
| WidgetStateProperty<Color>? get surfaceTintColor => | |
| const WidgetStatePropertyAll<Color>(Colors.transparent); |
| @override | ||
| WidgetStateProperty<Size>? get minimumSize => | ||
| const MaterialStatePropertyAll<Size>(Size(64.0, ${number(TokenButton.containerHeight)})); |
There was a problem hiding this comment.
Use WidgetStatePropertyAll instead of MaterialStatePropertyAll for minimumSize.
| @override | |
| WidgetStateProperty<Size>? get minimumSize => | |
| const MaterialStatePropertyAll<Size>(Size(64.0, ${number(TokenButton.containerHeight)})); | |
| @override | |
| WidgetStateProperty<Size>? get minimumSize => | |
| const WidgetStatePropertyAll<Size>(Size(64.0, ${number(TokenButton.containerHeight)})); |
| @override | ||
| WidgetStateProperty<double>? get iconSize => const MaterialStatePropertyAll<double>($_legacyIconSize); |
There was a problem hiding this comment.
Use WidgetStatePropertyAll instead of MaterialStatePropertyAll for iconSize.
| @override | |
| WidgetStateProperty<double>? get iconSize => const MaterialStatePropertyAll<double>($_legacyIconSize); | |
| @override | |
| WidgetStateProperty<double>? get iconSize => const WidgetStatePropertyAll<double>($_legacyIconSize); |
| @override | ||
| WidgetStateProperty<Size>? get maximumSize => const MaterialStatePropertyAll<Size>(Size.infinite); |
There was a problem hiding this comment.
Use WidgetStatePropertyAll instead of MaterialStatePropertyAll for maximumSize.
| @override | |
| WidgetStateProperty<Size>? get maximumSize => const MaterialStatePropertyAll<Size>(Size.infinite); | |
| @override | |
| WidgetStateProperty<Size>? get maximumSize => const WidgetStatePropertyAll<Size>(Size.infinite); |
| @override | ||
| WidgetStateProperty<OutlinedBorder>? get shape => | ||
| const MaterialStatePropertyAll<OutlinedBorder>(${shape(TokenButton.containerShapeRound, '')}); |
There was a problem hiding this comment.
Use WidgetStatePropertyAll instead of MaterialStatePropertyAll for shape.
| @override | |
| WidgetStateProperty<OutlinedBorder>? get shape => | |
| const MaterialStatePropertyAll<OutlinedBorder>(${shape(TokenButton.containerShapeRound, '')}); | |
| @override | |
| WidgetStateProperty<OutlinedBorder>? get shape => | |
| const WidgetStatePropertyAll<OutlinedBorder>(${shape(TokenButton.containerShapeRound, '')}); |
| expect( | ||
| contents, | ||
| contains('MaterialStatePropertyAll<TextStyle?>(Theme.of(context).textTheme.labelLarge)'), | ||
| ); |
There was a problem hiding this comment.
Update the test expectation to expect WidgetStatePropertyAll instead of MaterialStatePropertyAll to match the updated template.
| expect( | |
| contents, | |
| contains('MaterialStatePropertyAll<TextStyle?>(Theme.of(context).textTheme.labelLarge)'), | |
| ); | |
| expect( | |
| contents, | |
| contains('WidgetStatePropertyAll<TextStyle?>(Theme.of(context).textTheme.labelLarge)'), | |
| ); |
Work towards flutter/flutter#187899
Fixes flutter/flutter#188405
Pre-Review Checklist
[shared_preferences]///).