removed e.stopPropagation() from onKeyDown

[Anuragtech02]

Closes #2263

✅ Pull Request Checklist:

• stopPropagation error logs in useSelect #2263

📝 Test Instructions:

In order to test the PR, you can go the this link and try accessing the select box with arrow keys on the keyboard. Keep the terminal open along with this to check if it logs any errors.
This PR fixes the error mentioned in the issue.

[Anuragtech02]

Hey, @dannify I've simply deleted the stopPropagation function which in turn solves the issue for us! Let me know if I have to make any more changes.

[Anuragtech02]

Hey @dannify please let me know if this is okay

[LFDanLu]

@Anuragtech02 looks like one of the tests is failing (useMenuTrigger test). Mind mocking the isPropagationStopped call in that test to fix it?

    let isPropagationStopped = jest.fn();
     ....
    // triggers event if defaultPrevented is not true and it matches one of the keys
    menuTriggerProps.onKeyDown({
      pointerType: 'not keyboard',
      defaultPrevented: false,
      key: 'ArrowUp',
      preventDefault,
      stopPropagation,
      isPropagationStopped
    });
....

[Anuragtech02]

Sure, I'll give it a try!

[Anuragtech02]

Hey @LFDanLu, seems like some tests are still failing! I'm not able to understand what are they. Please let me know I'll fix those.

[LFDanLu]

LGTM. @Anuragtech02 the tests are passing on the commit, what test failures are you running into?

[Anuragtech02]

Surprisingly the tests passed! I don't know why but I did have one test which failed but now all are passing. Anyways, let me know if I have to make any more changes.

[snowystinger]

So I was just testing this out with yarn start:docs, I'm still seeing the warning. It seems to be because we spread the synthetic event into a new object https://github.com/adobe/react-spectrum/blob/main/packages/@react-aria/interactions/src/createEventHandler.ts#L27
this results in all getters on the event object being invoked, of which isPropagationStopped is one and it gets frozen as functionThatReturnsFalse
I think we just need one adjustment though.
Add this to the event in the createEventHandler

      isPropagationStopped(): boolean {
        return e.isPropagationStopped();
      }

UPDATE: Talked with Daniel some more, we've come to the conclusion that even with the change I requested here, it'll still have the same problem, due to the fact that we call stopPropagation AFTER the handler, meaning useMenuTrigger won't be able to check if propagation has been stopped yet. All that to say, let's just delete the two stopPropagation lines. Sorry for the confusion.

[Anuragtech02]

@snowystinger I'm sorry but I didn't quite get it. Am I supposed to remove the e.stopPropagation() line completely and not even check if propagation is stopped?

[snowystinger]

Correct. We're going to remove it. Reasoning being that you very likely need to usePress since we're passing press events around, and there are a few other things that just make it easier if you use it with react aria, so in all likelihood, we've already stopped propagation at this point. No need to check.

[LFDanLu]

LGTM, tested locally and confirmed that the warning no long appears. Apologies for the back and forth regarding the fix, and thanks for working on this!

[snowystinger]

lgtm, and as @LFDanLu said, thanks for the patience!