Skip to content

Commit

Permalink
Ensure Icon vertically centers its icon glyph. (#138937)
Browse files Browse the repository at this point in the history
Fixes flutter/flutter#138592. 

In an `Icon` widget if the icon font's body (ascender + descender) is larger than the font's units per em, the icon height reported by the text layout library will be larger than the specified font size. When that happens the icon glyph gets pushed towards the bottom because the `Icon` widget is wrapped in a fontSize x fontSize SizedBox and thus has a fixed height of fontSize px. This wasn't a problem for material icons because its UPEM == body.
  • Loading branch information
LongCatIsLooong authored Nov 28, 2023
1 parent 223f32e commit f2b7472
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 17 deletions.
39 changes: 22 additions & 17 deletions packages/flutter/lib/src/widgets/icon.dart
Original file line number Diff line number Diff line change
Expand Up @@ -250,6 +250,7 @@ class Icon extends StatelessWidget {

final List<Shadow>? iconShadows = shadows ?? iconTheme.shadows;

final IconData? icon = this.icon;
if (icon == null) {
return Semantics(
label: semanticLabel,
Expand All @@ -263,30 +264,34 @@ class Icon extends StatelessWidget {
iconColor = iconColor.withOpacity(iconColor.opacity * iconOpacity);
}

final TextStyle fontStyle = TextStyle(
fontVariations: <FontVariation>[
if (iconFill != null) FontVariation('FILL', iconFill),
if (iconWeight != null) FontVariation('wght', iconWeight),
if (iconGrade != null) FontVariation('GRAD', iconGrade),
if (iconOpticalSize != null) FontVariation('opsz', iconOpticalSize),
],
inherit: false,
color: iconColor,
fontSize: iconSize,
fontFamily: icon.fontFamily,
package: icon.fontPackage,
fontFamilyFallback: icon.fontFamilyFallback,
shadows: iconShadows,
height: 1.0, // Makes sure the font's body is vertically centered within the iconSize x iconSize square.
leadingDistribution: TextLeadingDistribution.even,
);

Widget iconWidget = RichText(
overflow: TextOverflow.visible, // Never clip.
textDirection: textDirection, // Since we already fetched it for the assert...
text: TextSpan(
text: String.fromCharCode(icon!.codePoint),
style: TextStyle(
fontVariations: <FontVariation>[
if (iconFill != null) FontVariation('FILL', iconFill),
if (iconWeight != null) FontVariation('wght', iconWeight),
if (iconGrade != null) FontVariation('GRAD', iconGrade),
if (iconOpticalSize != null) FontVariation('opsz', iconOpticalSize),
],
inherit: false,
color: iconColor,
fontSize: iconSize,
fontFamily: icon!.fontFamily,
package: icon!.fontPackage,
fontFamilyFallback: icon!.fontFamilyFallback,
shadows: iconShadows,
),
text: String.fromCharCode(icon.codePoint),
style: fontStyle,
),
);

if (icon!.matchTextDirection) {
if (icon.matchTextDirection) {
switch (textDirection) {
case TextDirection.rtl:
iconWidget = Transform(
Expand Down
16 changes: 16 additions & 0 deletions packages/flutter/test/widgets/icon_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,22 @@ void main() {
expect(richText.text.style!.fontFamily, equals('Roboto'));
});

testWidgetsWithLeakTracking("Icon's TextStyle makes sure the font body is vertically center-aligned", (WidgetTester tester) async {
// Regression test for https://github.com/flutter/flutter/issues/138592.
await tester.pumpWidget(
const Directionality(
textDirection: TextDirection.ltr,
child: Center(
child: Icon(IconData(0x41)),
),
),
);

final RichText richText = tester.firstWidget(find.byType(RichText));
expect(richText.text.style?.height, 1.0);
expect(richText.text.style?.leadingDistribution, TextLeadingDistribution.even);
});

testWidgetsWithLeakTracking('Icon with custom fontFamilyFallback', (WidgetTester tester) async {
await tester.pumpWidget(
const Directionality(
Expand Down

0 comments on commit f2b7472

Please sign in to comment.