Fix token usages on Regular Chip and Action Chip (#141701)

The regular chip and the action chip templates were referencing non existent M3 design tokens.

Fixes https://github.com/flutter/flutter/issues/141288

The `ActionChip` doesn't have any visual difference. Even though the template and file changes, the default `labelStyle` color already uses `onSurface`.
For the reviewer, I've changed the `action_chip_test` to expect a color from the colorScheme so that it is more explicit that the color might not be the same as the labelLarge default in the global textTheme, even if for this case the color is the same.

The regular `Chip` does have visual differences, in particular, the label and trailing icon colors, which were not following the specification. In order to fix this, the regular chip now is based from the `filter-chip` spec as described in the linked issue.

## Before

![image](https://github.com/flutter/flutter/assets/22084723/d602ef42-625a-4b5c-b63b-c46cb2070d80)

## After

![image](https://github.com/flutter/flutter/assets/22084723/dddb754f-fd29-4c4c-96cc-e7f508219f12)
This commit is contained in:
David Martos 2024-02-01 01:05:22 +01:00 committed by GitHub
parent ff6c8f5d37
commit b34ee07372
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
7 changed files with 40 additions and 18 deletions

View file

@ -1,6 +1,7 @@
Versions used, v0_162, v0_202, v0_158
md.comp.assist-chip.container.shape,
md.comp.assist-chip.container.surface-tint-layer.color,
md.comp.assist-chip.disabled.label-text.color,
md.comp.assist-chip.elevated.container.elevation,
md.comp.assist-chip.elevated.container.shadow-color,
md.comp.assist-chip.elevated.disabled.container.color,
@ -12,6 +13,7 @@ md.comp.assist-chip.flat.disabled.outline.color,
md.comp.assist-chip.flat.disabled.outline.opacity,
md.comp.assist-chip.flat.outline.color,
md.comp.assist-chip.flat.outline.width,
md.comp.assist-chip.label-text.color,
md.comp.assist-chip.label-text.text-style,
md.comp.assist-chip.with-icon.disabled.icon.color,
md.comp.assist-chip.with-icon.icon.color,

1 Versions used, v0_162, v0_202, v0_158
2 md.comp.assist-chip.container.shape,
3 md.comp.assist-chip.container.surface-tint-layer.color,
4 md.comp.assist-chip.disabled.label-text.color,
5 md.comp.assist-chip.elevated.container.elevation,
6 md.comp.assist-chip.elevated.container.shadow-color,
7 md.comp.assist-chip.elevated.disabled.container.color,
13 md.comp.assist-chip.flat.disabled.outline.opacity,
14 md.comp.assist-chip.flat.outline.color,
15 md.comp.assist-chip.flat.outline.width,
16 md.comp.assist-chip.label-text.color,
17 md.comp.assist-chip.label-text.text-style,
18 md.comp.assist-chip.with-icon.disabled.icon.color,
19 md.comp.assist-chip.with-icon.icon.color,

View file

@ -38,7 +38,11 @@ class _${blockName}DefaultsM3 extends ChipThemeData {
double? get pressElevation => ${elevation("$tokenGroup$elevatedVariant.pressed.container")};
@override
TextStyle? get labelStyle => ${textStyle("$tokenGroup.label-text")};
TextStyle? get labelStyle => ${textStyle("$tokenGroup.label-text")}?.copyWith(
color: isEnabled
? ${color("$tokenGroup.label-text.color")}
: ${color("$tokenGroup.disabled.label-text.color")},
);
@override
MaterialStateProperty<Color?>? get color =>
@ -60,10 +64,10 @@ class _${blockName}DefaultsM3 extends ChipThemeData {
Color? get surfaceTintColor => ${colorOrTransparent("$tokenGroup.container.surface-tint-layer.color")};
@override
Color? get checkmarkColor => ${color("$tokenGroup.with-icon.selected.icon.color")};
Color? get checkmarkColor => null;
@override
Color? get deleteIconColor => ${color("$tokenGroup.with-icon.selected.icon.color")};
Color? get deleteIconColor => null;
@override
BorderSide? get side => _chipVariant == _ChipVariant.flat

View file

@ -10,7 +10,7 @@ class ChipTemplate extends TokenTemplate {
super.textThemePrefix = '_textTheme.'
});
static const String tokenGroup = 'md.comp.assist-chip';
static const String tokenGroup = 'md.comp.filter-chip';
static const String variant = '.flat';
@override
@ -29,7 +29,11 @@ class _${blockName}DefaultsM3 extends ChipThemeData {
late final TextTheme _textTheme = Theme.of(context).textTheme;
@override
TextStyle? get labelStyle => ${textStyle("$tokenGroup.label-text")};
TextStyle? get labelStyle => ${textStyle("$tokenGroup.label-text")}?.copyWith(
color: isEnabled
? ${color("$tokenGroup.unselected.label-text.color")}
: ${color("$tokenGroup.disabled.label-text.color")},
);
@override
MaterialStateProperty<Color?>? get color => null; // Subclasses override this getter
@ -41,21 +45,23 @@ class _${blockName}DefaultsM3 extends ChipThemeData {
Color? get surfaceTintColor => ${colorOrTransparent("$tokenGroup.container.surface-tint-layer.color")};
@override
Color? get checkmarkColor => ${color("$tokenGroup.with-icon.selected.icon.color")};
Color? get checkmarkColor => null;
@override
Color? get deleteIconColor => ${color("$tokenGroup.with-icon.selected.icon.color")};
Color? get deleteIconColor => isEnabled
? ${color("$tokenGroup.with-trailing-icon.unselected.trailing-icon.color")}
: ${color("$tokenGroup.with-trailing-icon.disabled.trailing-icon.color")};
@override
BorderSide? get side => isEnabled
? ${border('$tokenGroup$variant.outline')}
: ${border('$tokenGroup$variant.disabled.outline')};
? ${border('$tokenGroup$variant.unselected.outline')}
: ${border('$tokenGroup$variant.disabled.unselected.outline')};
@override
IconThemeData? get iconTheme => IconThemeData(
color: isEnabled
? ${color("$tokenGroup.with-icon.icon.color")}
: ${color("$tokenGroup.with-icon.disabled.icon.color")},
? ${color("$tokenGroup.with-leading-icon.unselected.leading-icon.color")}
: ${color("$tokenGroup.with-leading-icon.disabled.leading-icon.color")},
size: ${getToken("$tokenGroup.with-icon.icon.size")},
);

View file

@ -259,7 +259,11 @@ class _ActionChipDefaultsM3 extends ChipThemeData {
double? get pressElevation => 1.0;
@override
TextStyle? get labelStyle => _textTheme.labelLarge;
TextStyle? get labelStyle => _textTheme.labelLarge?.copyWith(
color: isEnabled
? _colors.onSurface
: _colors.onSurface,
);
@override
MaterialStateProperty<Color?>? get color =>

View file

@ -2279,7 +2279,11 @@ class _ChipDefaultsM3 extends ChipThemeData {
late final TextTheme _textTheme = Theme.of(context).textTheme;
@override
TextStyle? get labelStyle => _textTheme.labelLarge;
TextStyle? get labelStyle => _textTheme.labelLarge?.copyWith(
color: isEnabled
? _colors.onSurfaceVariant
: _colors.onSurface,
);
@override
MaterialStateProperty<Color?>? get color => null; // Subclasses override this getter
@ -2294,7 +2298,9 @@ class _ChipDefaultsM3 extends ChipThemeData {
Color? get checkmarkColor => null;
@override
Color? get deleteIconColor => null;
Color? get deleteIconColor => isEnabled
? _colors.onSurfaceVariant
: _colors.onSurface;
@override
BorderSide? get side => isEnabled

View file

@ -155,7 +155,7 @@ void main() {
// Test default label style.
expect(
getLabelStyle(tester, label).style.color!.value,
theme.textTheme.labelLarge!.color!.value,
theme.colorScheme.onSurface.value,
);
Material chipMaterial = getMaterial(tester);
@ -229,7 +229,7 @@ void main() {
// Test default label style.
expect(
getLabelStyle(tester, label).style.color!.value,
theme.textTheme.labelLarge!.color!.value,
theme.colorScheme.onSurface.value,
);
Material chipMaterial = getMaterial(tester);

View file

@ -333,7 +333,7 @@ void main() {
expect(getIconData(tester).size, 18);
TextStyle labelStyle = getLabelStyle(tester, 'Chip A').style;
expect(labelStyle.color, textTheme.labelLarge?.color);
expect(labelStyle.color, lightTheme.colorScheme.onSurfaceVariant);
expect(labelStyle.fontFamily, textTheme.labelLarge?.fontFamily);
expect(labelStyle.fontFamilyFallback, textTheme.labelLarge?.fontFamilyFallback);
expect(labelStyle.fontFeatures, textTheme.labelLarge?.fontFeatures);
@ -361,7 +361,7 @@ void main() {
expect(getIconData(tester).size, 18);
labelStyle = getLabelStyle(tester, 'Chip A').style;
expect(labelStyle.color, textTheme.labelLarge?.color);
expect(labelStyle.color, darkTheme.colorScheme.onSurfaceVariant);
expect(labelStyle.fontFamily, textTheme.labelLarge?.fontFamily);
expect(labelStyle.fontFamilyFallback, textTheme.labelLarge?.fontFamilyFallback);
expect(labelStyle.fontFeatures, textTheme.labelLarge?.fontFeatures);