Skip to content

Commit

Permalink
Fix default avatar icon theme size for Material 2 (#152307)
Browse files Browse the repository at this point in the history 
fixes [ActionChip avatar still does not respect padding](flutter/flutter#116508)

### Code sample

<details>
<summary>expand to view the code sample</summary> 

```dart
import 'package:flutter/material.dart';

void main() => runApp(const MyApp());

class MyApp extends StatelessWidget {
  const MyApp({super.key});

  @OverRide
  Widget build(BuildContext context) {
    return MaterialApp(
      theme: ThemeData(brightness: Brightness.dark, useMaterial3: false),
      home: Scaffold(
        body: Center(
          child: Builder(
            builder: (BuildContext context) {
              return Chip(
                padding: const EdgeInsets.all(16.0),
                avatar: const Icon(Icons.favorite),
                label: const Text('Chip A'),
                onDeleted: () {},
              );
            },
          ),
        ),
      ),
    );
  }
}
```

</details>

### Before

<img width="490" alt="Screenshot 2024-07-25 at 16 24 08" src="https://github.com/user-attachments/assets/45408aa2-b3ab-4ff9-ae72-53a91c87c76a">

### After

<img width="490" alt="Screenshot 2024-07-25 at 16 23 56" src="https://github.com/user-attachments/assets/07ba367d-9ca3-46cc-8122-d1155dd2f32b">
  • Loading branch information
@TahaTesser
TahaTesser authored Jul 30, 2024
1 parent 61848a4 commit cd60efc
Show file tree
Hide file tree
Showing 4 changed files with 16 additions and 89 deletions.
2 changes: 1 addition & 1 deletion packages/flutter/lib/src/material/chip.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1369,7 +1369,7 @@ class _RawChipState extends State<RawChip> with MaterialStateMixin, TickerProvid
final TextStyle resolvedLabelStyle = effectiveLabelStyle.copyWith(color: resolvedLabelColor);
final Widget? avatar = iconTheme != null && hasAvatar
? IconTheme.merge(
data: theme.useMaterial3 ? chipDefaults.iconTheme!.merge(iconTheme) : iconTheme,
data: chipDefaults.iconTheme!.merge(iconTheme),
child: widget.avatar!,
)
: widget.avatar;
Expand Down
1 change: 1 addition & 0 deletions packages/flutter/lib/src/material/chip_theme.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -276,6 +276,7 @@ class ChipThemeData with Diagnosticable {
brightness: brightness,
elevation: 0.0,
pressElevation: 8.0,
iconTheme: const IconThemeData(size: 18.0),
);
}

Expand Down
91 changes: 7 additions & 84 deletions packages/flutter/test/material/chip_test.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -231,83 +231,6 @@ Finder findTooltipContainer(String tooltipText) {
}

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

Widget buildFrame(Brightness brightness) {
return MaterialApp(
theme: ThemeData(brightness: brightness, useMaterial3: false),
home: Scaffold(
body: Center(
child: Builder(
builder: (BuildContext context) {
textTheme = Theme.of(context).textTheme;
return Chip(
avatar: const CircleAvatar(child: Text('A')),
label: const Text('Chip A'),
onDeleted: () { },
);
},
),
),
),
);
}

await tester.pumpWidget(buildFrame(Brightness.light));
expect(getMaterialBox(tester), paints..rrect()..circle(color: const Color(0xff1976d2)));
expect(tester.getSize(find.byType(Chip)), const Size(156.0, 48.0));
expect(getMaterial(tester).color, null);
expect(getMaterial(tester).elevation, 0);
expect(getMaterial(tester).shape, const StadiumBorder());
expect(getIconData(tester).color?.value, 0xffffffff);
expect(getIconData(tester).opacity, null);
expect(getIconData(tester).size, null);

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

await tester.pumpWidget(buildFrame(Brightness.dark));
await tester.pumpAndSettle(); // Theme transition animation
expect(getMaterialBox(tester), paints..rrect(color: const Color(0x1fffffff)));
expect(tester.getSize(find.byType(Chip)), const Size(156.0, 48.0));
expect(getMaterial(tester).color, null);
expect(getMaterial(tester).elevation, 0);
expect(getMaterial(tester).shape, const StadiumBorder());
expect(getIconData(tester).color?.value, 0xffffffff);
expect(getIconData(tester).opacity, null);
expect(getIconData(tester).size, null);

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

testWidgets('M3 Chip defaults', (WidgetTester tester) async {
late TextTheme textTheme;
final ThemeData lightTheme = ThemeData.light();
Expand Down Expand Up @@ -4484,7 +4407,7 @@ void main() {
expect(box.size, equals(const Size(128, 32.0 + 16.0)));
expect(textBox.size, equals(const Size(56, 14)));
expect(iconBox.size, equals(const Size(18, 18)));
expect(avatarBox.size, equals(const Size(24, 24)));
expect(avatarBox.size, equals(const Size(18, 18)));
expect(textBox.top, equals(17));
expect(box.bottom - textBox.bottom, equals(17));
expect(textBox.left, equals(372));
Expand All @@ -4499,7 +4422,7 @@ void main() {
expect(box.size, equals(const Size(128, 60)));
expect(textBox.size, equals(const Size(56, 14)));
expect(iconBox.size, equals(const Size(18, 18)));
expect(avatarBox.size, equals(const Size(24, 24)));
expect(avatarBox.size, equals(const Size(18, 18)));
expect(textBox.top, equals(23));
expect(box.bottom - textBox.bottom, equals(23));
expect(textBox.left, equals(372));
Expand All @@ -4514,7 +4437,7 @@ void main() {
expect(box.size, equals(const Size(128, 36)));
expect(textBox.size, equals(const Size(56, 14)));
expect(iconBox.size, equals(const Size(18, 18)));
expect(avatarBox.size, equals(const Size(24, 24)));
expect(avatarBox.size, equals(const Size(18, 18)));
expect(textBox.top, equals(11));
expect(box.bottom - textBox.bottom, equals(11));
expect(textBox.left, equals(372));
Expand All @@ -4531,7 +4454,7 @@ void main() {
expect(box.size, equals(const Size(128, 36)));
expect(textBox.size, equals(const Size(56, 14)));
expect(iconBox.size, equals(const Size(18, 18)));
expect(avatarBox.size, equals(const Size(24, 24)));
expect(avatarBox.size, equals(const Size(18, 18)));
expect(textBox.top, equals(11));
expect(box.bottom - textBox.bottom, equals(11));
expect(textBox.left, equals(372));
Expand Down Expand Up @@ -5489,9 +5412,9 @@ void main() {
expect(getMaterial(tester).color, null);
expect(getMaterial(tester).elevation, 0);
expect(getMaterial(tester).shape, const StadiumBorder());
expect(getIconData(tester).color?.value, 0xffffffff);
expect(getIconData(tester).color, const Color(0xdd000000));
expect(getIconData(tester).opacity, null);
expect(getIconData(tester).size, null);
expect(getIconData(tester).size, 18.0);

TextStyle labelStyle = getLabelStyle(tester, 'Chip A').style;
expect(labelStyle.color?.value, 0xde000000);
Expand All @@ -5518,7 +5441,7 @@ void main() {
expect(getMaterial(tester).shape, const StadiumBorder());
expect(getIconData(tester).color?.value, 0xffffffff);
expect(getIconData(tester).opacity, null);
expect(getIconData(tester).size, null);
expect(getIconData(tester).size, 18.0);

labelStyle = getLabelStyle(tester, 'Chip A').style;
expect(labelStyle.color?.value, 0xdeffffff);
Expand Down
11 changes: 7 additions & 4 deletions packages/flutter/test/material/chip_theme_test.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -613,6 +613,7 @@ void main() {
selectedShadowColor: Colors.black,
showCheckmark: false,
checkmarkColor: Colors.black,
iconTheme: const IconThemeData(size: 26.0),
);
final ChipThemeData chipThemeWhite = ChipThemeData.fromDefaults(
secondaryColor: Colors.white,
Expand All @@ -630,6 +631,7 @@ void main() {
selectedShadowColor: Colors.white,
showCheckmark: true,
checkmarkColor: Colors.white,
iconTheme: const IconThemeData(size: 22.0),
);

final ChipThemeData lerp = ChipThemeData.lerp(chipThemeBlack, chipThemeWhite, 0.5)!;
Expand All @@ -653,7 +655,7 @@ void main() {
expect(lerp.elevation, 3.0);
expect(lerp.pressElevation, 7.0);
expect(lerp.checkmarkColor, equals(middleGrey));
expect(lerp.iconTheme, isNull);
expect(lerp.iconTheme, const IconThemeData(size: 24.0));

expect(ChipThemeData.lerp(null, null, 0.25), isNull);

Expand All @@ -677,7 +679,7 @@ void main() {
expect(lerpANull25.elevation, 1.25);
expect(lerpANull25.pressElevation, 2.5);
expect(lerpANull25.checkmarkColor, equals(Colors.white.withAlpha(0x40)));
expect(lerp.iconTheme, isNull);
expect(lerpANull25.iconTheme, const IconThemeData(size: 5.5));

final ChipThemeData lerpANull75 = ChipThemeData.lerp(null, chipThemeWhite, 0.75)!;
expect(lerpANull75.backgroundColor, equals(Colors.black.withAlpha(0x17)));
Expand All @@ -699,6 +701,7 @@ void main() {
expect(lerpANull75.elevation, 3.75);
expect(lerpANull75.pressElevation, 7.5);
expect(lerpANull75.checkmarkColor, equals(Colors.white.withAlpha(0xbf)));
expect(lerpANull75.iconTheme, const IconThemeData(size: 16.5));

final ChipThemeData lerpBNull25 = ChipThemeData.lerp(chipThemeBlack, null, 0.25)!;
expect(lerpBNull25.backgroundColor, equals(Colors.white.withAlpha(0x17)));
Expand All @@ -720,7 +723,7 @@ void main() {
expect(lerpBNull25.elevation, 0.75);
expect(lerpBNull25.pressElevation, 3.0);
expect(lerpBNull25.checkmarkColor, equals(Colors.black.withAlpha(0xbf)));
expect(lerp.iconTheme, isNull);
expect(lerpBNull25.iconTheme, const IconThemeData(size: 19.5));

final ChipThemeData lerpBNull75 = ChipThemeData.lerp(chipThemeBlack, null, 0.75)!;
expect(lerpBNull75.backgroundColor, equals(Colors.white.withAlpha(0x08)));
Expand All @@ -742,7 +745,7 @@ void main() {
expect(lerpBNull75.elevation, 0.25);
expect(lerpBNull75.pressElevation, 1.0);
expect(lerpBNull75.checkmarkColor, equals(Colors.black.withAlpha(0x40)));
expect(lerp.iconTheme, isNull);
expect(lerpBNull75.iconTheme, const IconThemeData(size: 6.5));
});

testWidgets('Chip uses stateful color from chip theme', (WidgetTester tester) async {
Expand Down

0 comments on commit cd60efc

Please sign in to comment.