react-spectrum
react-spectrum copied to clipboard
adobe
RAC Popover cannot be conditionally rendered via controlled props
Provide a general summary of the issue here
Popover will not render via a variable outside of its internal scope. I need to be able to control the open state of the popover so i can use framer motion to animate the entry/exit animation of the component. This is currently impossible.
🤔 Expected Behavior?
Popover should be able to be conditionally rendered outside of its internal state.
😯 Current Behavior
A simple wrapper over the Popover works perfectly fine, however when a predicate is set outside of the component to determine if it should render or not, it never renders. in fact the open/onOpenChange state never changes. In the code sample below, the issue is the state?.isOpen predicate. Even in the absence of framer motion, this predicate prevents the popover from rendering at all. The popover itself is permanently set to true so framer motion can control the entry/exit animation. Code sandbox is provided below.
<AnimatePresence>
{state?.isOpen && (
<MotionPopover
animate={{ opacity: 1 }}
initial={{ opacity: 0 }}
exit={{ opacity: 0 }}
isOpen
onOpenChange={state.setOpen}
style={{
background: "#e2e8f0",
padding: "12px",
border: "1px solid #94a3b8",
}}
>
{props.children}
</MotionPopover>
)}
</AnimatePresence>
💁 Possible Solution
No idea. ive mixed and matched context and state till ive been blue in the face.
🔦 Context
The original discussion that sparked this issue can be found at #5947. I originally thought the issue was due to something framer motions AnimatePresence was doing. however on further inspection the issue persists even without any framer motion involvement.
🖥️ Steps to Reproduce
Ive provided a sandbox that was also referenced in the original discussion here
Version
1.2.0 (RAC)
What browsers are you seeing the problem on?
Chrome
If other, please specify.
No response
What operating system are you using?
Windows 11
🧢 Your Company/Team
No response
🕷 Tracking Issue
No response
@LFDanLu don't suppose theres been any update on this or any related issues that'll also fix this? I can see this got taken off the component milestones...
No update on this that I'm aware of. The fastest way to get more priority on it would be to do some debugging in the source code. Anything that shortens the amount of time the team needs to look at it, the more likely we are to pick it up.
I'm not sure why it was added and removed so quickly from the milestones. It might have been added by mistake or optimistically. Apologies for the confusion there.
is there a decent readme for forking and getting going with the project locally? ive gotten as far as forking and get hit with a tonne of parcel errors, @spectrum-icons 'Cannot use import statement outside a module' errors, etc. i can only assume there's a combination of scripts i need to prebuild or something. im runnin on little to no sleep for the last 48 hours so if someone could explain this to me like im 5 id be grateful
No worries. First question would be, are you on windows. If so, there's a PR to address known build issues. You can work off it if need be.
Otherwise, steps are, fork and clone. Run yarn install, run yarn start. That should be it.
Every now and then you may need to 'rm -rf .parcel-cache' if it misbehaves during development.
CI uses Node v18.17.1 Yarn 1.20
@binaryartifex Sorry about that, I had added it to the milestone at first but we'll need to do some investigation on the root cause and a path forward before we add it there.
@LFDanLu so i done some more digging, so it looks like something in the Popover element needs to be in the dom at the time the calling component is rendered. easy way to test this is just remove the popover entirely from an RAC select component and then try logging the onOpenChange callback on the Select component itself. nothing happens. In the context of FramerMotion, AnimatePresence controls the exit/entry of components too and from the dom given a predicate, in this instance the 'isOpen' state, but if its falsey on initial render, it never renders. Would this have something to do with the way you guys assign refs and id's behind the scenes by checking for the existence of components to begin with?
@binaryartifex Ah, thank you for digging and for the reminder. I might have a lead, I believe it is due to this line: https://github.com/adobe/react-spectrum/blob/fabca84b95c9c61f9062d5f1e66ebe2c920a2a5d/packages/%40react-stately/select/src/useSelectState.ts#L73-L76. The reason that line is related is because the Select's collection is tied to the ListBox's collection. We rely on rendering a hidden copy of the Select's children so that the ListBox's collection can still be generated and provided to the Select state hooks but if the Popover isn't being rendered via conditional rendering that means we don't build the hidden ListBox and thus don't have a collection of items to use, resulting in that if check above skipping calling triggerState.open at all
I'm not very familiar with FramerMotion but I assume it needs the Popover not to be rendered in order for the animation to work properly? Somehow we'd need the Popover (and thus the ListBox) to be still rendered in our fake DOM to generate the collection regardless of state.isOpen but have that separated from the actual rendered state in the real DOM for FramerMotion.
It's probably a complete hack and I don't know if this has any negative impact on other things, but the select works if I duplicate a hidden Popover with children outside of AnimatePresence e.g.:
<>
<AnimatePresence>
{state?.isOpen && (
<MotionPopover
{...props}
animate={{ opacity: 1 }}
initial={{ opacity: 0 }}
exit={{ opacity: 0 }}
isOpen
onOpenChange={state.setOpen}
style={{
background: "#e2e8f0",
padding: "12px",
border: "1px solid #94a3b8",
}}
>
{props.children}
</MotionPopover>
)}
</AnimatePresence>
<Popover>
<div hidden>
{props.children}
</div>
</Popover>
</>
I have the same issue when using Menu with Popover. It gets stuck on the first frame.
I've opened a discussion here with a link to my CodeSandbox example. https://github.com/adobe/react-spectrum/discussions/7170
It would be great if this could get some attention.
