This pull request has been changed to a draft. The currently pending flutter-gold status will not be able to resolve until a new commit is pushed or the change is marked ready for review again.

For more guidance, visit Writing a golden file test for package:flutter.

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

@HansMuller This is the solution I came up for #132377. It breaks some existing tests because it adds a delay before the handleTap function is called.
If your ok with this approach, I will figure out where to add some documentation.

I've asked @gspencergoog to look at this. Introducing a delay before calling handleTap does seem fraught, at least on the face of it.

So, if I understand this correctly, this is basically adding a 200ms delay when an InkWell is activated via keyboard to simulate the time between the mouse down and mouse up of a tap?

Yes, exactly. I think the fundamental question is what does the 'pressed' state means when an Inkwell is activated via keyboard?
Before this PR, once activated via keyboard, the InkWell switch back immediately to pressed state false.

I know 200ms is arbitrary, so could it be 100ms instead? Or, even better, could it call handleTap right away, but execute the ink splash simultaneously?

Based on your comment, I updated the PR:

• to use a 100ms delay.
• to not rely on the existing handleTap (which is tailored for tap events) and introduce a separate logic for keyboard activation. Doing so neither the splash nor the onTap callback are delayed. So what is delayed is the MaterialStateController.value having the pressed state removed.

With this change it is way less risky (no existing test broken). Another solution would be to document that relying on MaterialState.pressed is not possible when using keyboard activation (it might not be easy to find where to add such a comment).

I think this is much better that the previous version.

We shouldn't just document that you can't rely on MaterialState.pressed from the keyboard, because that makes it harder to write code that works with both regular taps and keyboard activation, which should follow similar code paths.

I think, ideally, if you have keyboard activation, then it should enter the pressed state when the activation key goes down and activates the onTap, and exit the pressed state when it gets a key up, . Unfortunately, the ShortcutActivators only activate on key down, and ignore key up, so I'm not sure how you could implement that in a generic way (for instance, you could listen for key up events on Enter to turn off the state, but that isn't generic enough).

You could probably do something more complex and capture all the keys that were down when the InkWell was activated, and then remove the pressed state when any of those keys is no longer down, which would let you do it without knowing which one was the activator, but wouldn't be terribly accurate. In most cases, it would probably work well enough, though, since the common case would be a single key down (e.g. Enter or Space).

We can start with just the 100ms delay here, and if that becomes problematic (e.g. someone is writing a game and wants to know how long the key was pressed, but is using the pressed state and InkWells to do it), then we can revisit that decision. There are other ways to capture key events, so it likely will be good enough for most uses.