Skip to content
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

RAC Popover cannot be conditionally rendered via controlled props #6369

Open
binaryartifex opened this issue May 13, 2024 · 5 comments
Open

Comments

@binaryartifex
Copy link

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

@binaryartifex
Copy link
Author

@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...

@snowystinger
Copy link
Member

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.

@binaryartifex
Copy link
Author

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

@snowystinger
Copy link
Member

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

@LFDanLu
Copy link
Member

LFDanLu commented May 23, 2024

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants