Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Corrected a ChipTheme labelStyle corner case #94818

Merged
merged 1 commit into from
Dec 8, 2021
Merged

Corrected a ChipTheme labelStyle corner case #94818

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
11 changes: 6 additions & 5 deletions packages/flutter/lib/src/material/chip.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,8 @@ abstract class ChipAttributes {

/// The style to be applied to the chip's label.
///
/// If null, the value of the [ChipTheme]'s [ChipThemeData.labelStyle] is used.
/// The default label style is [TextTheme.bodyText1] from the overall
/// theme's [ThemeData.textTheme].
//
/// This only has an effect on widgets that respect the [DefaultTextStyle],
/// such as [Text].
Expand Down Expand Up @@ -1898,15 +1899,15 @@ class _RawChipState extends State<RawChip> with MaterialStateMixin, TickerProvid
?? chipTheme.padding
?? theme.chipTheme.padding
?? chipDefaults.padding!;
final TextStyle? labelStyle = widget.labelStyle
?? chipTheme.labelStyle
?? theme.chipTheme.labelStyle;
final TextStyle labelStyle = chipTheme.labelStyle
?? theme.chipTheme.labelStyle
?? chipDefaults.labelStyle!;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When can this be null?

Copy link
Contributor Author

@HansMuller HansMuller Dec 7, 2021 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ChipThemeData is now like the other theme data classes: all properties are null by default. You set them to override the default that's computed by the component.

The value of chipDefaults.labelStyle! can't be null ofcourse, chipDefaults is the internal representation of the default values for most properties.

final EdgeInsetsGeometry labelPadding = widget.labelPadding
?? chipTheme.labelPadding
?? theme.chipTheme.labelPadding
?? _defaultLabelPadding;

final TextStyle effectiveLabelStyle = chipDefaults.labelStyle!.merge(labelStyle);
final TextStyle effectiveLabelStyle = labelStyle.merge(widget.labelStyle);
final Color? resolvedLabelColor = MaterialStateProperty.resolveAs<Color?>(effectiveLabelStyle.color, materialStates);
final TextStyle resolvedLabelStyle = effectiveLabelStyle.copyWith(color: resolvedLabelColor);

Expand Down
95 changes: 88 additions & 7 deletions packages/flutter/test/material/chip_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -271,15 +271,22 @@ Finder findTooltipContainer(String tooltipText) {

void main() {
testWidgets('Chip defaults', (WidgetTester tester) async {
late TextTheme textTheme;

Widget buildFrame(Brightness brightness) {
return MaterialApp(
theme: ThemeData(brightness: brightness),
home: Scaffold(
body: Center(
child: Chip(
avatar: const CircleAvatar(child: Text('A')),
label: const Text('Chip A'),
onDeleted: () { },
child: Builder(
builder: (BuildContext context) {
textTheme = Theme.of(context).textTheme;
return Chip(
avatar: const CircleAvatar(child: Text('A')),
label: const Text('Chip A'),
onDeleted: () { },
);
},
),
),
),
Expand All @@ -295,7 +302,22 @@ void main() {
expect(getIconData(tester).color?.value, 0xffffffff);
expect(getIconData(tester).opacity, null);
expect(getIconData(tester).size, null);
expect(getLabelStyle(tester, 'Chip A').style.color?.value, 0xde000000);

TextStyle labelStyle = getLabelStyle(tester, 'Chip A').style;
expect(labelStyle.color?.value, 0xde000000);
expect(labelStyle.fontFamily, textTheme.bodyText1?.fontFamily);
expect(labelStyle.fontFamilyFallback, textTheme.bodyText1?.fontFamilyFallback);
expect(labelStyle.fontFeatures, textTheme.bodyText1?.fontFeatures);
expect(labelStyle.fontSize, textTheme.bodyText1?.fontSize);
expect(labelStyle.fontStyle, textTheme.bodyText1?.fontStyle);
expect(labelStyle.fontWeight, textTheme.bodyText1?.fontWeight);
expect(labelStyle.height, textTheme.bodyText1?.height);
expect(labelStyle.inherit, textTheme.bodyText1?.inherit);
expect(labelStyle.leadingDistribution, textTheme.bodyText1?.leadingDistribution);
expect(labelStyle.letterSpacing, textTheme.bodyText1?.letterSpacing);
expect(labelStyle.overflow, textTheme.bodyText1?.overflow);
expect(labelStyle.textBaseline, textTheme.bodyText1?.textBaseline);
expect(labelStyle.wordSpacing, textTheme.bodyText1?.wordSpacing);

await tester.pumpWidget(buildFrame(Brightness.dark));
await tester.pumpAndSettle(); // Theme transition animation
Expand All @@ -307,7 +329,22 @@ void main() {
expect(getIconData(tester).color?.value, 0xffffffff);
expect(getIconData(tester).opacity, null);
expect(getIconData(tester).size, null);
expect(getLabelStyle(tester, 'Chip A').style.color?.value, 0xdeffffff);

labelStyle = getLabelStyle(tester, 'Chip A').style;
expect(labelStyle.color?.value, 0xdeffffff);
expect(labelStyle.fontFamily, textTheme.bodyText1?.fontFamily);
expect(labelStyle.fontFamilyFallback, textTheme.bodyText1?.fontFamilyFallback);
expect(labelStyle.fontFeatures, textTheme.bodyText1?.fontFeatures);
expect(labelStyle.fontSize, textTheme.bodyText1?.fontSize);
expect(labelStyle.fontStyle, textTheme.bodyText1?.fontStyle);
expect(labelStyle.fontWeight, textTheme.bodyText1?.fontWeight);
expect(labelStyle.height, textTheme.bodyText1?.height);
expect(labelStyle.inherit, textTheme.bodyText1?.inherit);
expect(labelStyle.leadingDistribution, textTheme.bodyText1?.leadingDistribution);
expect(labelStyle.letterSpacing, textTheme.bodyText1?.letterSpacing);
expect(labelStyle.overflow, textTheme.bodyText1?.overflow);
expect(labelStyle.textBaseline, textTheme.bodyText1?.textBaseline);
expect(labelStyle.wordSpacing, textTheme.bodyText1?.wordSpacing);
});

testWidgets('ChoiceChip defaults', (WidgetTester tester) async {
Expand Down Expand Up @@ -343,7 +380,6 @@ void main() {
expect(getLabelStyle(tester, 'Chip A').style.color?.value, 0xdeffffff);
});


testWidgets('Chip control test', (WidgetTester tester) async {
final FeedbackTester feedback = FeedbackTester();
final List<String> deletedChipLabels = <String>[];
Expand Down Expand Up @@ -1726,10 +1762,55 @@ void main() {
await tester.pumpWidget(buildChip());

final TextStyle labelStyle = getLabelStyle(tester, 'Label').style;
expect(labelStyle.inherit, false);
expect(labelStyle.fontFamily, 'MyFont');
expect(labelStyle.fontWeight, FontWeight.w200);
});

testWidgets('ChipTheme labelStyle with inherit:true', (WidgetTester tester) async {
Widget buildChip() {
return _wrapForChip(
child: Theme(
data: ThemeData.light().copyWith(
chipTheme: const ChipThemeData(
labelStyle: TextStyle(height: 4), // inherit: true
),
),
child: const Chip(label: Text('Label')), // labeStyle: null
),
);
}

await tester.pumpWidget(buildChip());
final TextStyle labelStyle = getLabelStyle(tester, 'Label').style;
expect(labelStyle.inherit, true); // because chipTheme.labelStyle.merge(null)
expect(labelStyle.height, 4);
});

testWidgets('Chip does not merge inherit:false label style with the theme label style', (WidgetTester tester) async {
Widget buildChip() {
return _wrapForChip(
child: Theme(
data: ThemeData(fontFamily: 'MyFont'),
child: const DefaultTextStyle(
style: TextStyle(height: 8),
child: Chip(
label: Text('Label'),
labelStyle: TextStyle(fontWeight: FontWeight.w200, inherit: false),
),
),
),
);
}

await tester.pumpWidget(buildChip());
final TextStyle labelStyle = getLabelStyle(tester, 'Label').style;
expect(labelStyle.inherit, false);
expect(labelStyle.fontFamily, null);
expect(labelStyle.height, null);
expect(labelStyle.fontWeight, FontWeight.w200);
});

testWidgets('Chip size is configurable by ThemeData.materialTapTargetSize', (WidgetTester tester) async {
final Key key1 = UniqueKey();
await tester.pumpWidget(
Expand Down