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

Add menu dismiss localization #128613

Merged
merged 11 commits into from
Jul 19, 2023
Merged

Add menu dismiss localization #128613

merged 11 commits into from
Jul 19, 2023

Conversation

gspencergoog
Copy link
Contributor

@gspencergoog gspencergoog commented Jun 9, 2023 •
edited
Loading

Description

Modifies the semantic label for popup and context menus to be "Dismiss menu" instead of just "Dismiss".

Related Issues

Tests

  • Updated tests

@github-actions github-actions bot added a: internationalization Supporting other languages or locales. (aka i18n) a: text input Entering text in a text field or keyboard related problems f: cupertino flutter/packages/flutter/cupertino repository f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels. labels Jun 9, 2023
@gspencergoog gspencergoog marked this pull request as ready for review June 9, 2023 23:05
justinmc
justinmc approved these changes Jun 9, 2023
View reviewed changes
Copy link
Contributor

@justinmc justinmc left a comment

Choose a reason for hiding this comment

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

LGTM 👍 Thanks for fixing this.

@gspencergoog gspencergoog added the autosubmit Merge PR when tree becomes green via auto submit App label Jun 9, 2023
@auto-submit auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Jun 10, 2023
@auto-submit
Copy link
Contributor

auto-submit bot commented Jun 10, 2023

auto label is removed for flutter/flutter, pr: 128613, due to - The status or check suite Google testing has failed. Please fix the issues identified (or deflake) before re-applying this label.

tgucio
tgucio reviewed Jun 10, 2023
View reviewed changes
packages/flutter/lib/src/cupertino/localizations.dart Outdated
@@ -457,6 +461,9 @@ class DefaultCupertinoLocalizations implements CupertinoLocalizations {
@override
String get modalBarrierDismissLabel => 'Dismiss';

@override
String get contextMenuDismissLabel => 'Dismiss context menu';
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm thinking from a regular user's perspective maybe "Dismiss menu" would fit better than "Dismiss context menu"? Not sure how many people would call it a "context menu" rather than just "menu" - and how translatable "context menu" is.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Alright, that works for me.

@gspencergoog gspencergoog force-pushed the add_context_localization branch 5 times, most recently from c34c217 to 8084f20 Compare June 13, 2023 17:57
@gspencergoog gspencergoog changed the title Add context menu dismiss localization Add menu dismiss localization Jun 13, 2023
@gspencergoog gspencergoog added the autosubmit Merge PR when tree becomes green via auto submit App label Jun 14, 2023
@auto-submit auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Jun 14, 2023
@auto-submit
Copy link
Contributor

auto-submit bot commented Jun 14, 2023

auto label is removed for flutter/flutter, pr: 128613, due to - The status or check suite Linux framework_tests_libraries has failed. Please fix the issues identified (or deflake) before re-applying this label.

@gspencergoog gspencergoog added the autosubmit Merge PR when tree becomes green via auto submit App label Jun 14, 2023
@gspencergoog
Copy link
Contributor Author

I'll need to update the Google semantics tests so that they pass with the new value. I'll do that in July when I return from vacation.

@auto-submit auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Jul 7, 2023
@auto-submit
Copy link
Contributor

auto-submit bot commented Jul 7, 2023

auto label is removed for flutter/flutter, pr: 128613, due to - The status or check suite Linux analyze has failed. Please fix the issues identified (or deflake) before re-applying this label.

@gspencergoog gspencergoog force-pushed the add_context_localization branch 2 times, most recently from 8f9f5a8 to b8b5335 Compare July 18, 2023 15:36
@gspencergoog gspencergoog added the autosubmit Merge PR when tree becomes green via auto submit App label Jul 19, 2023
@auto-submit auto-submit bot merged commit 258382c into flutter:master Jul 19, 2023
73 checks passed
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Jul 20, 2023
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 20, 2023
@engine-flutter-autoroll
258382c59 Add menu dismiss localization (flutter/flutter#128613)
2fbf798
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Jul 20, 2023
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 20, 2023
@engine-flutter-autoroll
258382c59 Add menu dismiss localization (flutter/flutter#128613)
b2dd031
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Jul 20, 2023
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 20, 2023
@engine-flutter-autoroll
258382c59 Add menu dismiss localization (flutter/flutter#128613)
1102f9f
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Jul 21, 2023
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 21, 2023
@engine-flutter-autoroll
258382c59 Add menu dismiss localization (flutter/flutter#128613)
4f9e237
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 21, 2023
@engine-flutter-autoroll
258382c59 Add menu dismiss localization (flutter/flutter#128613)
8945c4d
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tgucio tgucio tgucio left review comments

@justinmc justinmc justinmc approved these changes

Assignees
No one assigned
Labels
a: internationalization Supporting other languages or locales. (aka i18n) a: text input Entering text in a text field or keyboard related problems autosubmit Merge PR when tree becomes green via auto submit App f: cupertino flutter/packages/flutter/cupertino repository f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Correct accessibility label for dismissing menus
4 participants
@gspencergoog @justinmc @tgucio @flutter-pub-roller-bot