Handle on outside clicks in reverse order for overlay #1251
Conversation
🦋 Changeset detectedLatest commit: 49a1190 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
This pull request is being automatically deployed with Vercel (learn more). 🔍 Inspect: https://vercel.com/primer/primer-components/AJTRgWD4eKmb26knEd7t5zFtR5UU |
src/hooks/useOnOutsideClick.tsx
Outdated
| if (handlers.length === 0) { | ||
| document.addEventListener('click', handleClick) | ||
| } | ||
| setTimeout(() => handlers.push(onOutsideClickInternal)) |
There was a problem hiding this comment.
😭 This is the only way I can get things to work - otherwise the outside click event for the DropdownModal fires at the same time that the dropdown opens.
|
@dgreif @VanAnderson Would y’all mind taking a look at the edits in 2b2f5e8? I synchronously reviewed them with @dgreif already, but I’m a bit wary to approve and merge this since I contributed to that refactor. |
|
@smockle you changes look great. I'm comfortable merging this PR at this point. We can always circle back with refactors if needed, but the current code both solves the problem and doesn't have side-effects like the first couple iterations that were dependent on memoized props. |
Transplant logic from onEscapePress to onOutsideClick - this allows us to have Overlays inside of overlays that will respect proper close behavior!