Open sue-hit opened 3 years ago
@sue-hit Good catch, looks like this is because of this block of code . useDialog
performs a blur + refocus to try and move Voiceover cursor onto the Dialog itself but this means that the dialog gets closed due to the shouldCloseOnBlur
passed to useOverlay.
I'm trying to think of an appropriate way of resolving this, but it definitely feels like we should lean towards fixing that block so it doesn't break overlays with shouldCloseOnBlur
@sue-hit Good catch, looks like this is because of this block of code .
useDialog
performs a blur + refocus to try and move Voiceover cursor onto the Dialog itself but this means that the dialog gets closed due to theshouldCloseOnBlur
passed to useOverlay.I'm trying to think of an appropriate way of resolving this, but it definitely feels like we should lean towards fixing that block so it doesn't break overlays with
shouldCloseOnBlur
Just wanted to confirm it is this block of code. I was able to fix with a local patch commenting that block out.
@LFDanLu what is the correct way to fix this? Happy to send a PR.
I'm not sure, open to ideas. I haven't been able to spend any time investigating this since my last comment unfortunately, but the solution will need to maintain the current Voiceover cursor behavior
I'm not getting a crash, but a bug that when opening overlay it's closed after a moment.
This is because useDialog
triggers blur that causes shouldCloseOnBlur
to close the overlay
I managed to find a workaround that relies on newly released code in react-aria@3.20.0
calling event.stopPropagation
in dialogProps
's onBlur
. It's not the best solution for me, but one that works for now
const { overlayProps } = useOverlay(
{
isOpen,
onClose,
shouldCloseOnBlur: true,
isDismissable: true,
},
popoverRef
)
const { dialogProps } = useDialog({}, popoverRef)
const { onBlur, ...overlayPropsWithoutOnBlur } = overlayProps
// this onBlur will never be called because div inside of it calls `event.stopPropagation` when refocusing
<div onBlur={onBlur}>
<div
ref={popoverRef}
{...mergeProps(dialogProps, overlayPropsWithoutOnBlur)}
>
{children}
</div>
</div>
🐛 Bug Report
I'm trying to create a popover component with react-aria, but setting
shouldCloseOnBlur: true
results in getting the above error when the popover is opened. I have a minimal failing example below, in which the only thing different from the useOverlayTrigger example is the shouldCloseOnBlur: true (https://react-spectrum.adobe.com/react-aria/useOverlayTrigger.html)🤔 Expected Behavior
I'm hoping to be able to supply shouldCloseOnBlur without the code crashing
😯 Current Behavior
TypeError Cannot read property 'focus' of null at focusWithoutScrolling (https://tfwh9.csb.app/node_modules/ react-aria/utils/dist/module.js:140:13 at focusSafely (https://tfwh9.csb.app/node_modules/ react-aria/focus/dist/module.js:32:43 at eval (https://tfwh9.csb.app/node_modules/ react-aria/dialog/dist/module.js:22:39
💁 Possible Solution
As far as I can tell from my actual app, this code seems to be called a few times, first with the popover trigger, then maybe its enclosing elements ... then body, then null. But before the error, the popover does appear momentarily - so maybe just checking if the element exists in focusWithoutScrolling might make this go away?
🔦 Context
I'd like to migrate a popover component I have to use react-aria (for more reliable behaviour, and it would give me keyboard handling on the component <3) but this issue is stopping me.
💻 Code Sample
https://codesandbox.io/s/youthful-dawn-tfwh9?file=/src/App.js
🌍 Your Environment