-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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.
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
feat: event leak demo #7376
base: main
Are you sure you want to change the base?
feat: event leak demo #7376
Conversation
It would be great if someone could issue a build for this 👍 |
Just chiming in, I believe I actually just ran into this event leak behavior myself in Autocomplete since I was dispatching fake keyboard events to the wrapped collection which bubbled up to the dialog and closed it prematurely. I'll make a duplicate of this branch since security dinged us on running builds on fork PRs and see about getting the team to take a look as soon as possible |
For others looking at the PR, here is a build link: https://reactspectrum.blob.core.windows.net/reactspectrum/0b6cec88ba545ce9c6f8086c28bb33d07752ece3/storybook/index.html?path=/story/react-aria-components--event-leak-grid&providerSwitcher-express=false |
@nwidynski Apologies for the delay, the team discussed this some more yesterday and moving forward we'll want to be more strict/consistent about stopping propagation on keyboard events. This usually happens by default when calling This change will hopefully make handing interactive components within a grid much easier as well as possibly allowing the typeahead listener to become a non-capturing listener. |
@LFDanLu Got it, I generally very much agree with the sentiment to have atomic building blocks. Let's hope this doesn't cause too many behavior issues in user applications 🙏 As this effort will likely take a while, we will scratch our immediate need by beginning work on open-sourcing a custom grid virtualizer, which prevents these leaks from reaching the collection 👍 |
Closes comment
TLDR; I did not test for every possible interaction for now and also limited the assertions to grid interaction keys only. It's very likely that additional components will also fail when tested more throughly. Point of this demo was to hightlight that many components, not only nested collections, will cause problems when used inside grids.
Also related are #7370 and my latest comment on #7321.
✅ Pull Request Checklist:
📝 Test Instructions:
Event Leak Grid
story🧢 Your Project: