@yjbanov ?

Bot tagged as web because there are two SemanticsFlag, one on the web and one everywhere else. I don't know who is responsible, lol. This could fall under accessibility. But Greg likes to review my PRs, if he is on engine and Yegor doesn't fit, you could assign him.

Ps. I saw some tests are failing. I'm into it. Just figuring out how to get gclient sync to work, this is tricky.
Ps2. I'm really struggling to get things working.
Ps3. The issue is probably getDartClassFields returning invalid because it is an enum.

/cc @chunhtai @Hangyujin

It looks like ci is failing, can you fix it?

Yes, but I need your help. Getting gsync to work is too much for me. Can you help in Discord? I don't know how to get api_check to appear.

based on some of the error message, you may need to do a migration

Try correcting the name to the name of an existing getter, or defining a getter or field named 'values'.
      for (final SemanticsAction action in SemanticsAction.values.values)
                                                                  ^^^^^^
lib/src/semantics/semantics.dart:683:61: Error: The getter 'values' isn't defined for the class 'List<SemanticsFlag>'.
 - 'List' is from 'dart:core'.
 - 'SemanticsFlag' is from 'dart:ui'.
Try correcting the name to the name of an existing getter, or defining a getter or field named 'values'.
      for (final SemanticsFlag flag in SemanticsFlag.values.values)
                                                            ^^^^^^
lib/src/semantics/semantics.dart:2758:53: Error: The getter 'values' isn't defined for the class 'List<SemanticsFlag>'.
 - 'List' is from 'dart:core'.
 - 'SemanticsFlag' is from 'dart:ui'.
Try correcting the name to the name of an existing getter, or defining a getter or field named 'values'.
    final List<String> flags = SemanticsFlag.values.values.where((SemanticsFlag flag) => hasFlag(flag)).map((SemanticsFlag flag) => flag.toString().substring('SemanticsFlag.'.length)).toList();
                                                    ^^^^^^

Do I make the migration in this PR? Or do I make one in Flutter and tie them together? I thought the error message was something else. This one is easy. I have it here: flutter/flutter#123329

Unfortunately, this can't be migrate like this because engine and framework pr can't go in at the same time.

You would need to create an temporarily API so that engine and framework pr can go in separately.

I went ahead. This now is blocked by #40571 😀

I just rebased on top of master.

Do you think SemanticsAction.deserialize(flag) is a good replacement for SemanticsAction.values[flag]? AndroidSemanticsAction used "deserialize", which was my inspiration.

I'm also using value as the value. field or something else would also be valid names (even action, flag or actionId could be good names! But value works for both, so the cognitive load might be lower. It also works as the individual unit of values).

Looks like some checks are failing

Hold on, testing here (I need a YouTube tutorial on how to debug the engine lol). But almost there.

I have a marriage now, I'll probably only finish this on the weekend.

The test failing is SemanticsFlag enums match, probably getDartClassFields doesn't know how to get an enum, that's my guess. If you want to help, feel free, else I'll take a look later. Engine Ninja took 17min to compile here, hopefully that's it.

One test fixed. The next step is this. Does anyone know what might be happening? Is there a way Dart_GetNativeIntegerArgument is getting the wrong value because it is used to class instead of enum?

Tap seems correct, but action_id seems wrong, where is Dart_GetNativeIntegerArgument called? This is getting hard for me.

../../flutter/shell/platform/embedder/tests/embedder_a11y_unittests.cc:241: Failure
Expected equality of these values:
  static_cast<int32_t>(flutter::SemanticsAction::kTap)
    Which is: 1
  action_id
    Which is: 0

Update: that test seems to be 2 weeks old from @loic-sharma. Could you help me here? I have no idea what to do.

Hm, somewhere this is still failing. I wonder if there is another place where we accidentally use index instead of value....

Yeah, I think that is it: Things like this here

https://github.com/flutter/flutter/blob/73bd97852978a4778f51dae0c48bc0c08d674edb/packages/flutter/test/cupertino/button_test.dart#L291

are now doing the wrong thing because they use Enum.index instead of what we now call value. There may not be a good way to migrate this since I don't think we can override Enum.index.

Ahhhhhhhh... We kind of opened a pandora box, each day something new appears lol
I think I found a test that might fail (just commited), but that's a lot worse.

I guess... hmm.. add value to engine, replace index with value in Flutter and then make the migration?

What about user code that currently uses .index? They will be broken in the worst possible way: everything still appears to be valid and compiles, but then it may fail in hard-to-debug ways at runtime. I am actually more leaning towards not doing this clean-up now.

I agree. But...are there users using SemanticsAction? I searched code search and found a single usage, which the person copied Flutter bindings for Go. I doesn't seem to have that big of a user-base. But we could deprecate index, move to value, then migrate, or just don't do it. 99% of the (few) users use SemanticsAction.tap, not SemanticsAction.tap.index.

We could make half migration, deprecate index, move to value, then in a few months/years remove the value and put the enum in place without anyone seeing.

You could:

a) add value to the SemanticsAction class (not the enum)
b) migrate the framework
c) remove index from the SemanticsAction class and see if any of our customer tests (or google testing) break. If nothing breaks, we can continue with this migration and change the class to an enum. If things break, we may have to undo all that.

It's upto you if you're up for doing all that extra work :)

Well, we came until here, we can get there (as long as your patience allows)... value is 1 LOC. Let's try and see.

Hixie will get tired of pressing ignore tests.

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat (don't just cc him here, he won't see it! He's on Discord!).

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

After a lot of discussion and nice ideas, this will be the end result. It is not an enum, and might probably never be, but it is almost there.

Do you prefer from, actionAt, fromAction, fromIndex or deserialize for the => values[index] getter? Somewhere else in Flutter uses deserialize, but I don't like. from or fromAction sounds better because SemanticsAction.fromAction(action) is more readable than SemanticsAction.fromIndex(action). But anything is fine.

Thanks for all the help!

Golden file changes have been found for this pull request. Click here to view and triage (e.g. because this is an intentional change).

If you are still iterating on this change and are not ready to resolve the images on the Flutter Gold dashboard, consider marking this PR as a draft pull request above. You will still be able to view image results on the dashboard, commenting will be silenced, and the check will not try to resolve itself until marked ready for review.

Changes reported for pull request #40567 at sha eb423ae

The bot went crazy.

Thanks, everybody for this long journey. Feel free to add the "autosubmit".