-
Notifications
You must be signed in to change notification settings - Fork 29.8k
Cupertino text clear label #129727
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.
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
Cupertino text clear label #129727
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the label should be localized?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll put that in.
0e10c2d to
0099223
Compare
LongCatIsLooong
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not familiar with semantics or internationalization. @chunhtai @thkim1011 do you mind taking a look?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Much of these arb files don't seem to have the correct translation for "clearButtonLabel", "menuDismissLabel" or "noSpellCheckReplacementsLabel"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The process is that these arb files holds the english strings as a placeholder until they are translated internally. Then we can incorporate those translations into Flutter. Although, it looks like some arb files are missing some strings. Was this README followed for regenerating the arb files?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I found that README file partway through manually adding the clearButtonLabel to some of the arb files and was originally copying the flow from previous PRs adding localization. The only difference I saw was the placement of clearButtonLabel was different in the files I manually added. The other two translations are from other work.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are there arb files missing the translation besides the cupertino_xx_XX.arb files? I assumed the script skipped those for a reason.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah my bad, I thought menuDismissLabel was added in this PR but I think it just changed because of commas at the end of the line. The localization changes LGTM.
388c313 to
9af7989
Compare
9af7989 to
aa0fdf0
Compare
e858313 to
60e683d
Compare
60e683d to
b5ce6c9
Compare
b98ad08 to
3c91c5d
Compare
|
@MitchellGoodwin said last week that he still has plans to finish this. |
3c91c5d to
a479e8b
Compare
goderbauer
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
| /// The semantic label for the clear button used by screen readers. | ||
| /// | ||
| /// This will be used by screen reading software to identify the clear button | ||
| /// widget. Defaults to "Text Field Clear". |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From looking at the intl files: doesn't this just default to "clear"?
| CupertinoIcons.clear_thick_circled, | ||
| size: 18.0, | ||
| color: CupertinoDynamicColor.resolve(_kClearButtonColor, context), | ||
| final CupertinoLocalizations localizations = CupertinoLocalizations.of(context); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would do:
final String clearLabel = widget.clearButtonSemanticLabel ?? CupertinoLocalizations.of(context).clearButtonLabel;That way, you only establish a dependency on CupertinoLocalizations if you actually need it.
| ), | ||
| ); | ||
|
|
||
| expect(find.bySemanticsLabel('Text Field Clear'), findsNothing); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be
| expect(find.bySemanticsLabel('Text Field Clear'), findsNothing); | |
| expect(find.bySemanticsLabel('Clear'), findsNothing); |
flutter/flutter@a51e33a...2407f69 2023-12-15 58529443+srujzs@users.noreply.github.com Move package:web dependency to dev dependency (flutter/flutter#139696) 2023-12-15 engine-flutter-autoroll@skia.org Roll Flutter Engine from 9524a185b055 to 986a6fe198dc (1 revision) (flutter/flutter#140221) 2023-12-15 engine-flutter-autoroll@skia.org Roll Packages from 1151191 to 3f2e16b (9 revisions) (flutter/flutter#140218) 2023-12-15 engine-flutter-autoroll@skia.org Roll Flutter Engine from 7a50221733c2 to 9524a185b055 (1 revision) (flutter/flutter#140217) 2023-12-15 engine-flutter-autoroll@skia.org Roll Flutter Engine from 767223f7a4f8 to 7a50221733c2 (1 revision) (flutter/flutter#140216) 2023-12-15 engine-flutter-autoroll@skia.org Roll Flutter Engine from 91f65eea0c11 to 767223f7a4f8 (1 revision) (flutter/flutter#140210) 2023-12-15 engine-flutter-autoroll@skia.org Roll Flutter Engine from a47da28c9a62 to 91f65eea0c11 (1 revision) (flutter/flutter#140207) 2023-12-15 engine-flutter-autoroll@skia.org Roll Flutter Engine from cde1a596432d to a47da28c9a62 (1 revision) (flutter/flutter#140204) 2023-12-15 engine-flutter-autoroll@skia.org Roll Flutter Engine from 46ff5c08a905 to cde1a596432d (1 revision) (flutter/flutter#140200) 2023-12-15 engine-flutter-autoroll@skia.org Roll Flutter Engine from a17bb0a63b7e to 46ff5c08a905 (1 revision) (flutter/flutter#140198) 2023-12-15 engine-flutter-autoroll@skia.org Roll Flutter Engine from 4cb3ba7a85f6 to a17bb0a63b7e (1 revision) (flutter/flutter#140196) 2023-12-15 engine-flutter-autoroll@skia.org Roll Flutter Engine from 0e7248d43251 to 4cb3ba7a85f6 (14 revisions) (flutter/flutter#140195) 2023-12-15 polinach@google.com Increase versions of leak tracker libraries. (flutter/flutter#140018) 2023-12-15 magder@google.com Set compile test iOS app target version to not embed Swift runtime (flutter/flutter#140188) 2023-12-15 58190796+MitchellGoodwin@users.noreply.github.com Cupertino text clear label (flutter/flutter#129727) 2023-12-15 xilaizhang@google.com [github actions] use token from real user flutter mirror bot (flutter/flutter#140191) 2023-12-15 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Roll Flutter Engine from 0e7248d43251 to 0b0fab821536 (4 revisions)" (flutter/flutter#140194) 2023-12-14 engine-flutter-autoroll@skia.org Roll Flutter Engine from 0e7248d43251 to 0b0fab821536 (4 revisions) (flutter/flutter#140180) 2023-12-14 lsaudon@gmail.com feat: Add onTapAlwaysCalled in TextFormField (flutter/flutter#140089) 2023-12-14 49699333+dependabot[bot]@users.noreply.github.com Bump actions/upload-artifact from 3.1.3 to 4.0.0 (flutter/flutter#140177) 2023-12-14 engine-flutter-autoroll@skia.org Roll Flutter Engine from 2140942444ea to 0e7248d43251 (2 revisions) (flutter/flutter#140176) 2023-12-14 ybz975218925@gmail.com fix reorderable_list drop animation (flutter/flutter#139362) 2023-12-14 engine-flutter-autoroll@skia.org Roll Flutter Engine from 997d3dfa1e74 to 2140942444ea (4 revisions) (flutter/flutter#140171) 2023-12-14 38147403+sharmashashi@users.noreply.github.com Fix BottomNavigationBarItem label overflow (flutter/flutter#120206) 2023-12-14 engine-flutter-autoroll@skia.org Roll Flutter Engine from a565cea256c7 to 997d3dfa1e74 (2 revisions) (flutter/flutter#140170) 2023-12-14 chingjun@google.com Revert "Dynamic view sizing" (flutter/flutter#140165) 2023-12-14 aki.nishi.work@gmail.com �: fix cupertionActionSheet design (flutter/flutter#134345) 2023-12-14 104349824+huycozy@users.noreply.github.com Make improvements to existing new issue templates (flutter/flutter#140142) 2023-12-14 engine-flutter-autoroll@skia.org Roll Flutter Engine from caf33276468b to a565cea256c7 (1 revision) (flutter/flutter#140163) 2023-12-14 parlough@gmail.com Expand and update a few release.yml categories (flutter/flutter#140120) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages Please CC dit@google.com,rmistry@google.com,stuartmorgan@google.com on the revert to ensure that a human is aware of the problem. To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
Fixes #123107
Adds a customizable semantic label so that the clear button on the Cupertino text field will be picked up by screen readers.
RPReplay_Final1687901827.mov
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.