radix-ui / themes

Radix Themes is an open-source component library optimized for fast development, easy maintenance, and accessibility. Maintained by @workos.
https://radix-ui.com/themes
MIT License
5.45k stars 194 forks source link

[Popover] Issue: Controlled by state #343

Closed jasonsturges closed 7 months ago

jasonsturges commented 7 months ago

Can the Popover component be manually controlled?

As in, by state:

  const [open, setOpen] = useState(true);

  return <Popover.Root open={open} />

When the Popover.Trigger is present it works, but when removed it can no longer be controlled: popover https://stackblitz.com/edit/vitejs-vite-pp3q8h?file=src%2FApp.tsx

Dialog exhibits a similar pattern, but also there is no way to exclude the modal background.

Per the description, this is probably mandatory usage to be triggered by a button:

Floating element for displaying rich content, triggered by a button.

In my use case, there are informational boxes that relate to a map interface - there's a complex layer of business logic with very dynamic presentation.

There are also instructional / help boxes similar to https://introjs.com/ that are manually controlled.

With MUI, these are satisfied with Snackbars, Dialogs, and Speed Dials - with some generic Modal implementation.

Any recommendation of usage, or potential feature request if you see this as valid.

benoitgrelard commented 7 months ago

Yes it can be controlled but here you haven't wired onOpenChange, so it will never update. That's the same as wiring a <input value={value} /> without wiring the onChange.

Additionally, you do need to use the trigger part, otherwise it can't know where to position the content relatively to that, hence why it's not visible.

jasonsturges commented 7 months ago

Open is the React state:

  const [open, setOpen] = useState(true); // Controlled via state, not by Trigger

  return (
    <>
      <Button onClick={() => { setOpen(true) }}>Open</Button>
      <Button onClick={() => { setOpen(false) }}>Close</Button>

      <Popover.Root open={open} />

Just an example where buttons were controlling state.

Using the onOpenChange puts logic in the Popover, which I'm trying to do the exact opposite.

MUI and libraries like Bootstrap have anchor references, or other strategies for positioning but understood - positioning is the challenge here that doesn't fit elegantly into Radix UI Themes.

Thanks.

benoitgrelard commented 7 months ago

It doesn't put "logic" in the Popover, using open you're telling it when to be open. onOpenChange is the component telling you when it needs to be open/closed (ie. when the trigger is clicked, it will tell you to open it, when clicking outside the popover, it will tell you to close it). That's the principle of controlled components, same as a native input.

What are you trying to achieve here that you can't achieve with this?

jasonsturges commented 7 months ago

I want to programmatically control it, such as the use cases I mentioned.

Maybe the most basic example - just auto-close:

function App() {
  const [open, setOpen] = useState(true);

  setTimeout(() => {
    setOpen(false);
  }, 2500);

  return (
    <>
      <Theme appearance="dark">
        <Popover.Root open={open}>

popover2

Of course that timeout should be triggered by an onOpenChange handler, but just as an example... I don't want to use the trigger business logic at all.

I must be confusing this with MUI, which has a controlled component pattern such as:

image

https://mui.com/material-ui/react-dialog/

Don't mean to misuse the components - I'll adapt a custom component based on Popover or Dialog.

benoitgrelard commented 7 months ago

The example with the timer is correct, just missing wiring up onOpenChange so that it's also closable when clicking outside the popover.

Essentially, when comparing to the MUI one, they only expose a change handler as onClose presumably because they don't have the concept of using a dialog completely uncontrolled (so needing to open it from within, using a trigger part).

jasonsturges commented 7 months ago

Similar to: https://github.com/radix-ui/primitives/discussions/1234

thien-do commented 5 months ago

@jasonsturges Can you try passing an empty function to Popover's onOpenChange?

const [open, setOpen] = useState(false);

<Button onClick={() => setOpen(true)}>Open</Button>
<Button onClick={() => setOpen(false)}>Close</Button>

<Popover.Root open={open} onOpenChange={() => {}}>
  ...
</Popover>

This works in my case. The key here is that:

thien-do commented 5 months ago

@benoitgrelard There's one issue though, but maybe I should say feature request: We could not have "Anchor" like in Radix Primitives 🤔 I think at this point we should use Radix Primitives already 😅 But anyway this is the only missing piece.

Context here is that I'm opening a Popover next to, and with, a Dropdown. At the moment I need to wrap Popover's Trigger outside of Dropdown's Content, which feels weird because (1) Trigger is interactive and (2) it's supposed to be a button:

image

P.s. You folks must be tired from hearing this all day but I could not help repeating it again: Thanks for an excellent library design! This is the best API design on earth for a UI kit!

thien-do commented 5 months ago

Update on my comment above, in case anyone is looking for the same issue:

It's possible to use Popover.Anchor from Radix Primitives with Radix Themes to skip the Trigger:

image