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.

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

Prevent explicit roles from merging #164732

Merged
merged 3 commits into from
Mar 13, 2025
Merged

Conversation

chunhtai
Copy link
Contributor

@chunhtai chunhtai commented Mar 6, 2025

If merging explicit roles, some of the information may be dropped.

Pre-launch Checklist

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

@github-actions github-actions bot added framework flutter/packages/flutter repository. See also f: labels. a: accessibility Accessibility, e.g. VoiceOver or TalkBack. (aka a11y) labels Mar 6, 2025
@chunhtai chunhtai requested a review from yjbanov March 6, 2025 19:12
_hasFlag(SemanticsFlag.isLink) ||
_hasFlag(SemanticsFlag.scopesRoute) ||
_hasFlag(SemanticsFlag.isImage) ||
_hasFlag(SemanticsFlag.isKeyboardKey)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we also have isButton here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

no, an image can also be a button. same for keyboard key. button is more of a trait now...

now I think of it, there is also a searchBox/combobox which can be compatible with istextfield... it almost feels like the compatibility is per flag and role based..

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we may need to make the role and enhance enum to be able to enter compatible flags

Copy link
Contributor Author

Choose a reason for hiding this comment

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

discussed offline, for role that may be compatibile, we could (a) factor out some aspect of it to a semanticsproperty(e.g searchable) (b) comes up with a way to customize merging for each enum (e.g. an enhance enum that include other mergable roles).

we can deal with them case by case when these kind of problem raise up. For now, we will go ahead with this pr

@github-actions github-actions bot added the f: material design flutter/packages/flutter/material repository. label Mar 6, 2025
@chunhtai chunhtai added the autosubmit Merge PR when tree becomes green via auto submit App label Mar 6, 2025
Copy link
Contributor

auto-submit bot commented Mar 6, 2025

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

@auto-submit auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Mar 6, 2025
@chunhtai chunhtai added the autosubmit Merge PR when tree becomes green via auto submit App label Mar 6, 2025
@auto-submit auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Mar 6, 2025
Copy link
Contributor

auto-submit bot commented Mar 6, 2025

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

@chunhtai chunhtai added the autosubmit Merge PR when tree becomes green via auto submit App label Mar 7, 2025
@auto-submit auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Mar 10, 2025
Copy link
Contributor

auto-submit bot commented Mar 10, 2025

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

@chunhtai chunhtai added the autosubmit Merge PR when tree becomes green via auto submit App label Mar 13, 2025
@auto-submit auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Mar 13, 2025
Copy link
Contributor

auto-submit bot commented Mar 13, 2025

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

@chunhtai chunhtai added the autosubmit Merge PR when tree becomes green via auto submit App label Mar 13, 2025
@auto-submit auto-submit bot added this pull request to the merge queue Mar 13, 2025
Merged via the queue into flutter:master with commit c2bb3ac Mar 13, 2025
75 of 76 checks passed
@flutter-dashboard flutter-dashboard bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Mar 13, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 14, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 14, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 14, 2025
chunhtai added a commit to chunhtai/flutter that referenced this pull request Mar 14, 2025
github-merge-queue bot pushed a commit that referenced this pull request Mar 15, 2025
This reverts commit c2bb3ac.

<!--
Thanks for filing a pull request!
Reviewers are typically assigned within a week of filing a request.
To learn more about code review, see our documentation on Tree Hygiene:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md
-->

the previous pr may break some of the mobile a11y. some newly splitted
role can't be focus by mobile a11y focus, will need to fix some of the
engine code first

## Pre-launch Checklist

- [ ] I read the [Contributor Guide] and followed the process outlined
there for submitting PRs.
- [ ] I read the [Tree Hygiene] wiki page, which explains my
responsibilities.
- [ ] I read and followed the [Flutter Style Guide], including [Features
we expect every widget to implement].
- [ ] I signed the [CLA].
- [ ] I listed at least one issue that this PR fixes in the description
above.
- [ ] I updated/added relevant documentation (doc comments with `///`).
- [ ] I added new tests to check the change I am making, or this PR is
[test-exempt].
- [ ] I followed the [breaking change policy] and added [Data Driven
Fixes] where supported.
- [ ] All existing and new tests are passing.

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

<!-- Links -->
[Contributor Guide]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview
[Tree Hygiene]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md
[test-exempt]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests
[Flutter Style Guide]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md
[Features we expect every widget to implement]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#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/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes
[Discord]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md
[Data Driven Fixes]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 15, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 15, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 16, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 16, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 16, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 17, 2025
chunhtai added a commit to chunhtai/flutter that referenced this pull request Mar 17, 2025
chunhtai added a commit to chunhtai/flutter that referenced this pull request Mar 17, 2025
chunhtai added a commit to chunhtai/flutter that referenced this pull request Mar 17, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a: accessibility Accessibility, e.g. VoiceOver or TalkBack. (aka a11y) f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants