Skip to content

Fix Popup menu Semantics label for mismatched platforms - #177049

Merged
auto-submit[bot] merged 3 commits into
flutter:masterfrom
huycozy:fix-a11y-platform-theme-176566-popupmenu
Oct 28, 2025
Merged

Fix Popup menu Semantics label for mismatched platforms#177049
auto-submit[bot] merged 3 commits into
flutter:masterfrom
huycozy:fix-a11y-platform-theme-176566-popupmenu

Conversation

@huycozy

@huycozy huycozy commented Oct 15, 2025

Copy link
Copy Markdown
Member

Pre-launch Checklist

If you need help, consider asking for advice on the #hackers-new channel on Discord.

Note: The Flutter team is currently trialing the use of Gemini Code Assist for GitHub. Comments from the gemini-code-assist bot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed.

@huycozy huycozy self-assigned this Oct 15, 2025
@github-actions github-actions Bot added framework flutter/packages/flutter repository. See also f: labels. p: material_ui material_ui package in flutter/packages labels Oct 15, 2025
@huycozy

ghost commented Oct 15, 2025

Copy link
Copy Markdown
Member Author

/gemini review

gemini-code-assist[bot]
gemini-code-assist Bot reviewed Oct 15, 2025
View reviewed changes

ghost left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request correctly fixes an accessibility issue where the popup menu's semantics label was determined by the theme's platform instead of the actual device platform. The change in popup_menu.dart to use defaultTargetPlatform is appropriate and directly addresses the problem. The new tests in popup_menu_test.dart are well-written and effectively cover the regression scenarios for both Apple and non-Apple platforms, ensuring the fix works as intended. I've included one suggestion to refactor the new tests to improve code maintainability by reducing duplication.

Comment thread packages/flutter/test/material/popup_menu_test.dart Outdated
@huycozy
huycozy force-pushed the fix-a11y-platform-theme-176566-popupmenu branch from 10c1643 to dd8bab6 Compare October 16, 2025 04:49
@huycozy
huycozy marked this pull request as ready for review October 16, 2025 05:59
@huycozy
huycozy requested a review from chunhtai October 17, 2025 03:51
chunhtai
chunhtai reviewed Oct 17, 2025
View reviewed changes

ghost left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

Comment thread packages/flutter/test/material/popup_menu_test.dart Outdated
@huycozy
huycozy force-pushed the fix-a11y-platform-theme-176566-popupmenu branch from dd8bab6 to 5bb50f8 Compare October 23, 2025 15:58
@huycozy
huycozy requested a review from chunhtai October 23, 2025 15:59
chunhtai
chunhtai approved these changes Oct 23, 2025
View reviewed changes

ghost left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@huycozy huycozy added the autosubmit Merge PR when tree becomes green via auto submit App label Oct 24, 2025
@auto-submit auto-submit Bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Oct 24, 2025
@auto-submit

ghost commented Oct 24, 2025

Copy link
Copy Markdown
Contributor

autosubmit label was removed for flutter/flutter/177049, because - The status or check suite Google testing has failed. Please fix the issues identified (or deflake) before re-applying this label.

@huycozy
huycozy force-pushed the fix-a11y-platform-theme-176566-popupmenu branch from 5bb50f8 to 414204f Compare October 28, 2025 04:03
@huycozy huycozy added the autosubmit Merge PR when tree becomes green via auto submit App label Oct 28, 2025
@auto-submit
auto-submit Bot added this pull request to the merge queue Oct 28, 2025
Merged via the queue into flutter:master with commit c648d89 Oct 28, 2025
@flutter-dashboard flutter-dashboard Bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Oct 28, 2025
@huycozy
huycozy deleted the fix-a11y-platform-theme-176566-popupmenu branch October 28, 2025 06:56
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Oct 28, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@chunhtai chunhtai chunhtai approved these changes
+1 more reviewer
@gemini-code-assist gemini-code-assist[bot] gemini-code-assist[bot] left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

@huycozy huycozy

Labels

framework flutter/packages/flutter repository. See also f: labels. p: material_ui material_ui package in flutter/packages

Projects

Google Testing Queue
Status: Done

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[a11y] Use defaultTargetPlatform instead of platform from the theme for PopupMenu's semantics

2 participants

@huycozy @chunhtai