Skip to content

SemanticsFlag/SemanticsAction cleanup (part 4)#123329

Merged
auto-submit[bot] merged 4 commits intoflutter:masterfrom
bernaferrari:SemanticsFlag
Apr 12, 2023
Merged

SemanticsFlag/SemanticsAction cleanup (part 4)#123329
auto-submit[bot] merged 4 commits intoflutter:masterfrom
bernaferrari:SemanticsFlag

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