From 0919fb86f1b61727ebcc39ff6ea4203cbfce0381 Mon Sep 17 00:00:00 2001 From: Pierre-Louis <6655696+guidezpl@users.noreply.github.com> Date: Fri, 21 Jul 2023 19:07:39 +0200 Subject: [PATCH] Improve handling of certain icons in RTL (#130979) Fixes https://github.com/flutter/flutter/issues/130978 ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord]. [Contributor Guide]: https://github.com/flutter/flutter/wiki/Tree-hygiene#overview [Tree Hygiene]: https://github.com/flutter/flutter/wiki/Tree-hygiene [test-exempt]: https://github.com/flutter/flutter/wiki/Tree-hygiene#tests [Flutter Style Guide]: https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo [Features we expect every widget to implement]: https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/wiki/Tree-hygiene#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/wiki/Chat --- dev/tools/test/update_icons_test.dart | 57 ++++++++++++++++++++ dev/tools/update_icons.dart | 12 +++-- packages/flutter/lib/src/material/icons.dart | 40 +++++++------- 3 files changed, 84 insertions(+), 25 deletions(-) diff --git a/dev/tools/test/update_icons_test.dart b/dev/tools/test/update_icons_test.dart index b7fa706d93..6b1fbe4362 100644 --- a/dev/tools/test/update_icons_test.dart +++ b/dev/tools/test/update_icons_test.dart @@ -61,4 +61,61 @@ void main() { 'Icon(Icons.onetwothree_rounded),', ); }); + + test('certain icons should be mirrored in RTL', () { + // Exact match + expect( + Icon(const MapEntry('help', '')).isMirroredInRTL, + true, + ); + // Variant + expect( + Icon(const MapEntry('help_rounded', '')).isMirroredInRTL, + true, + ); + // Common suffixes + expect( + Icon(const MapEntry('help_alt', '')).isMirroredInRTL, + true, + ); + expect( + Icon(const MapEntry('help_new', '')).isMirroredInRTL, + true, + ); + expect( + Icon(const MapEntry('help_off', '')).isMirroredInRTL, + true, + ); + expect( + Icon(const MapEntry('help_on', '')).isMirroredInRTL, + true, + ); + // Common suffixes + variant + expect( + Icon(const MapEntry('help_alt_rounded', '')).isMirroredInRTL, + true, + ); + expect( + Icon(const MapEntry('help_new_rounded', '')).isMirroredInRTL, + true, + ); + expect( + Icon(const MapEntry('help_off_rounded', '')).isMirroredInRTL, + true, + ); + expect( + Icon(const MapEntry('help_on_rounded', '')).isMirroredInRTL, + true, + ); + // No match + expect( + Icon(const MapEntry('help_center_rounded', '')).isMirroredInRTL, + false, + ); + // No match + expect( + Icon(const MapEntry('arrow', '')).isMirroredInRTL, + false, + ); + }); } diff --git a/dev/tools/update_icons.dart b/dev/tools/update_icons.dart index bbe77f0459..9d56f540ba 100644 --- a/dev/tools/update_icons.dart +++ b/dev/tools/update_icons.dart @@ -150,7 +150,7 @@ const Set _iconsMirroredWhenRTL = { 'navigate_next', 'next_week', 'note', - 'open_in_new', + 'open_in', 'playlist_add', 'queue_music', 'redo', @@ -513,12 +513,14 @@ class Icon { String get usage => 'Icon($className.$flutterId),'; - String get mirroredInRTL => _iconsMirroredWhenRTL.contains(shortId) - ? ', matchTextDirection: true' - : ''; + bool get isMirroredInRTL { + // Remove common suffixes (e.g. "_new" or "_alt") from the shortId. + final String normalizedShortId = shortId.replaceAll(RegExp(r'_(new|alt|off|on)$'), ''); + return _iconsMirroredWhenRTL.any((String shortIdMirroredWhenRTL) => normalizedShortId == shortIdMirroredWhenRTL); + } String get declaration => - "static const IconData $flutterId = IconData(0x$hexCodepoint, fontFamily: '$fontFamily'$mirroredInRTL);"; + "static const IconData $flutterId = IconData(0x$hexCodepoint, fontFamily: '$fontFamily'${isMirroredInRTL ? ', matchTextDirection: true' : ''});"; String get fullDeclaration => ''' diff --git a/packages/flutter/lib/src/material/icons.dart b/packages/flutter/lib/src/material/icons.dart index 59f3b08849..5d96c9914d 100644 --- a/packages/flutter/lib/src/material/icons.dart +++ b/packages/flutter/lib/src/material/icons.dart @@ -2154,16 +2154,16 @@ abstract final class Icons { static const IconData arrow_back_ios_outlined = IconData(0xee84, fontFamily: 'MaterialIcons', matchTextDirection: true); /// arrow_back_ios_new — material icon named "arrow back ios new". - static const IconData arrow_back_ios_new = IconData(0xe094, fontFamily: 'MaterialIcons'); + static const IconData arrow_back_ios_new = IconData(0xe094, fontFamily: 'MaterialIcons', matchTextDirection: true); /// arrow_back_ios_new — material icon named "arrow back ios new" (sharp). - static const IconData arrow_back_ios_new_sharp = IconData(0xe791, fontFamily: 'MaterialIcons'); + static const IconData arrow_back_ios_new_sharp = IconData(0xe791, fontFamily: 'MaterialIcons', matchTextDirection: true); /// arrow_back_ios_new — material icon named "arrow back ios new" (round). - static const IconData arrow_back_ios_new_rounded = IconData(0xf570, fontFamily: 'MaterialIcons'); + static const IconData arrow_back_ios_new_rounded = IconData(0xf570, fontFamily: 'MaterialIcons', matchTextDirection: true); /// arrow_back_ios_new — material icon named "arrow back ios new" (outlined). - static const IconData arrow_back_ios_new_outlined = IconData(0xee83, fontFamily: 'MaterialIcons'); + static const IconData arrow_back_ios_new_outlined = IconData(0xee83, fontFamily: 'MaterialIcons', matchTextDirection: true); /// arrow_circle_down — material icon named "arrow circle down". static const IconData arrow_circle_down = IconData(0xe095, fontFamily: 'MaterialIcons'); @@ -2322,16 +2322,16 @@ abstract final class Icons { static const IconData arrow_right_outlined = IconData(0xee90, fontFamily: 'MaterialIcons', matchTextDirection: true); /// arrow_right_alt — material icon named "arrow right alt". - static const IconData arrow_right_alt = IconData(0xe09f, fontFamily: 'MaterialIcons'); + static const IconData arrow_right_alt = IconData(0xe09f, fontFamily: 'MaterialIcons', matchTextDirection: true); /// arrow_right_alt — material icon named "arrow right alt" (sharp). - static const IconData arrow_right_alt_sharp = IconData(0xe79d, fontFamily: 'MaterialIcons'); + static const IconData arrow_right_alt_sharp = IconData(0xe79d, fontFamily: 'MaterialIcons', matchTextDirection: true); /// arrow_right_alt — material icon named "arrow right alt" (round). - static const IconData arrow_right_alt_rounded = IconData(0xf57c, fontFamily: 'MaterialIcons'); + static const IconData arrow_right_alt_rounded = IconData(0xf57c, fontFamily: 'MaterialIcons', matchTextDirection: true); /// arrow_right_alt — material icon named "arrow right alt" (outlined). - static const IconData arrow_right_alt_outlined = IconData(0xee8f, fontFamily: 'MaterialIcons'); + static const IconData arrow_right_alt_outlined = IconData(0xee8f, fontFamily: 'MaterialIcons', matchTextDirection: true); /// arrow_upward — material icon named "arrow upward". static const IconData arrow_upward = IconData(0xe0a0, fontFamily: 'MaterialIcons'); @@ -12900,16 +12900,16 @@ abstract final class Icons { static const IconData label_important_outline_rounded = IconData(0xf839, fontFamily: 'MaterialIcons'); /// label_off — material icon named "label off". - static const IconData label_off = IconData(0xe363, fontFamily: 'MaterialIcons'); + static const IconData label_off = IconData(0xe363, fontFamily: 'MaterialIcons', matchTextDirection: true); /// label_off — material icon named "label off" (sharp). - static const IconData label_off_sharp = IconData(0xea5c, fontFamily: 'MaterialIcons'); + static const IconData label_off_sharp = IconData(0xea5c, fontFamily: 'MaterialIcons', matchTextDirection: true); /// label_off — material icon named "label off" (round). - static const IconData label_off_rounded = IconData(0xf83b, fontFamily: 'MaterialIcons'); + static const IconData label_off_rounded = IconData(0xf83b, fontFamily: 'MaterialIcons', matchTextDirection: true); /// label_off — material icon named "label off" (outlined). - static const IconData label_off_outlined = IconData(0xf14c, fontFamily: 'MaterialIcons'); + static const IconData label_off_outlined = IconData(0xf14c, fontFamily: 'MaterialIcons', matchTextDirection: true); /// label_outline — material icon named "label outline". static const IconData label_outline = IconData(0xe364, fontFamily: 'MaterialIcons', matchTextDirection: true); @@ -13362,16 +13362,16 @@ abstract final class Icons { static const IconData list_outlined = IconData(0xf16d, fontFamily: 'MaterialIcons', matchTextDirection: true); /// list_alt — material icon named "list alt". - static const IconData list_alt = IconData(0xe385, fontFamily: 'MaterialIcons'); + static const IconData list_alt = IconData(0xe385, fontFamily: 'MaterialIcons', matchTextDirection: true); /// list_alt — material icon named "list alt" (sharp). - static const IconData list_alt_sharp = IconData(0xea7e, fontFamily: 'MaterialIcons'); + static const IconData list_alt_sharp = IconData(0xea7e, fontFamily: 'MaterialIcons', matchTextDirection: true); /// list_alt — material icon named "list alt" (round). - static const IconData list_alt_rounded = IconData(0xf85d, fontFamily: 'MaterialIcons'); + static const IconData list_alt_rounded = IconData(0xf85d, fontFamily: 'MaterialIcons', matchTextDirection: true); /// list_alt — material icon named "list alt" (outlined). - static const IconData list_alt_outlined = IconData(0xf16c, fontFamily: 'MaterialIcons'); + static const IconData list_alt_outlined = IconData(0xf16c, fontFamily: 'MaterialIcons', matchTextDirection: true); /// live_help — material icon named "live help". static const IconData live_help = IconData(0xe386, fontFamily: 'MaterialIcons', matchTextDirection: true); @@ -16224,16 +16224,16 @@ abstract final class Icons { static const IconData note_add_outlined = IconData(0xf22e, fontFamily: 'MaterialIcons'); /// note_alt — material icon named "note alt". - static const IconData note_alt = IconData(0xe44b, fontFamily: 'MaterialIcons'); + static const IconData note_alt = IconData(0xe44b, fontFamily: 'MaterialIcons', matchTextDirection: true); /// note_alt — material icon named "note alt" (sharp). - static const IconData note_alt_sharp = IconData(0xeb42, fontFamily: 'MaterialIcons'); + static const IconData note_alt_sharp = IconData(0xeb42, fontFamily: 'MaterialIcons', matchTextDirection: true); /// note_alt — material icon named "note alt" (round). - static const IconData note_alt_rounded = IconData(0xf0021, fontFamily: 'MaterialIcons'); + static const IconData note_alt_rounded = IconData(0xf0021, fontFamily: 'MaterialIcons', matchTextDirection: true); /// note_alt — material icon named "note alt" (outlined). - static const IconData note_alt_outlined = IconData(0xf22f, fontFamily: 'MaterialIcons'); + static const IconData note_alt_outlined = IconData(0xf22f, fontFamily: 'MaterialIcons', matchTextDirection: true); /// notes — material icon named "notes". static const IconData notes = IconData(0xe44c, fontFamily: 'MaterialIcons');