Skip to content

SemanticsFlag/SemanticsAction cleanup (part 4) #123329

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
Merged
auto-submit merged 4 commits into from
Apr 12, 2023
Merged

SemanticsFlag/SemanticsAction cleanup (part 4) #123329

auto-submit merged 4 commits into from
Apr 12, 2023

Conversation

@bernaferrari
Copy link
Contributor

@bernaferrari bernaferrari commented Mar 23, 2023 •
edited
Loading

part 4 of #123346

@flutter-dashboard flutter-dashboard bot added a: accessibility Accessibility, e.g. VoiceOver or TalkBack. (aka a11y) a: tests "flutter test", flutter_test, or one of our tests framework flutter/packages/flutter repository. See also f: labels. labels Mar 23, 2023
@bernaferrari
Copy link
Contributor Author

bernaferrari commented Mar 23, 2023 •
edited
Loading

Ops, need to fix on Engine first.

@bernaferrari bernaferrari mentioned this pull request Mar 23, 2023
Merged
@bernaferrari bernaferrari changed the title Make SemanticsAction and SemanticsFlag become an enum. SemanticsFlag/SemanticsAction enum migration (part 4) Mar 23, 2023
@bernaferrari bernaferrari changed the title SemanticsFlag/SemanticsAction enum migration (part 4) SemanticsFlag/SemanticsAction enum migration (part 4) Mar 23, 2023
This was referenced Mar 23, 2023
Merged
Closed
@bernaferrari bernaferrari reopened this Mar 23, 2023
@goderbauer
Copy link
Member

goderbauer commented Mar 23, 2023

What about the matcher_test.dart in flutter_test that iterates over SemanticsAction.values.keys?

@bernaferrari
Copy link
Contributor Author

bernaferrari commented Mar 24, 2023 •
edited
Loading

That's an excellente question. I completely overlooked it. Thanks!

We could:

  • Have a public map with the values. My original idea was to copy _kActionById idea from AndroidSemanticsAction (the part 3 had it), but there it was private; we could make it public.
  • Add a @protected Iterable<int> keys => _kActionById.keys;, since that's used in tests only.

What do you think?

@bernaferrari
Copy link
Contributor Author

bernaferrari commented Mar 24, 2023

Oh, alternatively we could just do SemanticsAction.values.map((d) => d.value)) right? Would work the same.

@goderbauer
Copy link
Member

goderbauer commented Mar 24, 2023

SemanticsAction.values.map((d) => d.value)) sounds like the best option.

@bernaferrari bernaferrari force-pushed the SemanticsFlag branch 2 times, most recently from 191e9b1 to 25906f6 Compare April 11, 2023 01:08
bernaferrari
bernaferrari commented Apr 11, 2023
View reviewed changes
@bernaferrari bernaferrari force-pushed the SemanticsFlag branch 5 times, most recently from 19c87ce to b5ea0a1 Compare April 11, 2023 03:23
@bernaferrari bernaferrari marked this pull request as ready for review April 11, 2023 03:26
@bernaferrari bernaferrari changed the title SemanticsFlag/SemanticsAction enum migration (part 4) SemanticsFlag/SemanticsAction cleanup (part 4) Apr 11, 2023
@bernaferrari
Copy link
Contributor Author

bernaferrari commented Apr 11, 2023

Cc @chunhtai

goderbauer
goderbauer reviewed Apr 11, 2023
View reviewed changes
@chunhtai chunhtai self-requested a review April 11, 2023 16:30
goderbauer
goderbauer approved these changes Apr 11, 2023
View reviewed changes
Copy link
Member

@goderbauer goderbauer left a comment

Choose a reason for hiding this comment

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

LGTM

@goderbauer
Copy link
Member

goderbauer commented Apr 11, 2023

FYI, Google testing is currently failing because the engine-side change has not rolled into google yet.

chunhtai
chunhtai approved these changes Apr 11, 2023
View reviewed changes
Copy link
Contributor

@chunhtai chunhtai left a comment

Choose a reason for hiding this comment

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

LGTM

@bernaferrari bernaferrari force-pushed the SemanticsFlag branch 2 times, most recently from 3b08912 to 5d9942e Compare April 12, 2023 04:13
@bernaferrari
Copy link
Contributor Author

bernaferrari commented Apr 12, 2023

I'll use my new super-power to auto-submit (I hope it is fine).

@bernaferrari bernaferrari added the autosubmit Merge PR when tree becomes green via auto submit App label Apr 12, 2023
@auto-submit auto-submit bot merged commit 2e4d976 into flutter:master Apr 12, 2023
@bernaferrari bernaferrari deleted the SemanticsFlag branch April 12, 2023 19:07
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Apr 13, 2023
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Apr 13, 2023
@engine-flutter-autoroll
2e4d976 SemanticsFlag/SemanticsAction cleanup (part 4) (flutter/f…
c248a46 
…lutter#123329)
exaby73 pushed a commit to exaby73/flutter_nevercode that referenced this pull request Apr 17, 2023
@bernaferrari @exaby73
SemanticsFlag/SemanticsAction cleanup (part 4) (flutter#123329)
0585a28 
`SemanticsFlag`/`SemanticsAction` cleanup (part 4)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@goderbauer goderbauer goderbauer approved these changes

@chunhtai chunhtai chunhtai approved these changes

Assignees

No one assigned

Labels

a: accessibility Accessibility, e.g. VoiceOver or TalkBack. (aka a11y) a: tests "flutter test", flutter_test, or one of our tests autosubmit Merge PR when tree becomes green via auto submit App 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.

3 participants

@bernaferrari @goderbauer @chunhtai