Unexpected behavior when composing Popover and Tooltip on the same element

[bengry]

Current behavior

Using a Tooltip and a Popover on the same element doesn't always works as expected - both show up when hovering the anchor element.

This only seems to happen based on the order of the providers:
broken:

 <Ariakit.PopoverProvider>
  <Ariakit.TooltipProvider>{content}</Ariakit.TooltipProvider>
</Ariakit.PopoverProvider>

working:

<Ariakit.TooltipProvider>
	<Ariakit.PopoverProvider>{content}</Ariakit.PopoverProvider>
</Ariakit.TooltipProvider>

I guess this is because of the way these two interact internally, so they share the same store even though they should have entirely separate ones, but I'm not sure.

Steps to reproduce the bug

https://stackblitz.com/edit/vitejs-vite-iuggxs?file=src%2FApp.tsx,src%2Fstyle.css

Expected behavior

The order of providers shouldn't matter in this case.

Workaround

Change around the Providers' order such that:

<Ariakit.TooltipProvider>
	<Ariakit.PopoverProvider>
		{content}
	</Ariakit.PopoverProvider>
</Ariakit.TooltipProvider>

Or #3754 (comment)

Possible solutions

Give each usage of a provider type a name (e.g. "TooltipHovercardStore") such that even if two components use the same underlying primitives (e.g. Hovercard), they won't share stores since they consume it using separate names.

[bengry]

@diegohaz thanks for adding the tag. Just to add some more context on this - we're seeing some odd, probably related, behavior to this in our app - the Popover just doesn't open if the composition is such that the <PopoverProvider> is above the <TooltipProvider>. This is especially annoying since it's less readable, not to mention less convenient to use, with a simple abstraction on top of the Tooltip component:

Example app code:

<Popover>
  <Popover.Content>...</Popover.Content>

  <Popover.Trigger>
    <Tooltip content="more info">
      <Button>i</Button>
    </Tooltip>
  </Popover.Trigger>
</Popover>

Under the hood this roughly translates to this implementation:
https://stackblitz.com/edit/vitejs-vite-wmwcpz?file=src%2FApp.tsx

It makes a lot more sense to keep the Tooltip wrapping the button, rather than above the entire Popover, the content of which might be long.

If you think that this is a separate bug/issue (maybe because of the <Slot>s? but I don't think so), LMK and I'll open another issue on GitHub, but any help on this would be much appreciated.

[diegohaz]

If you think that this is a separate bug/issue (maybe because of the <Slot>s? but I don't think so), LMK and I'll open another issue on GitHub, but any help on this would be much appreciated.

It indeed appears to be the same problem. Thanks for the reproductions.

FWIW, you don't need Slot. You can just pass children to render:

-  <Ariakit.PopoverDisclosure ref={ref} render={<Slot />}>
-    {children}
-  </Ariakit.PopoverDisclosure>
+ <Ariakit.PopoverDisclosure ref={ref} render={children} />

This also forces you to improve type safety because you'll need to type children as ReactElement, not ReactNode. The latter could cause a runtime error in Slot.

[bengry]

FWIW, you don't need Slot. You can just pass children to render:

-  <Ariakit.PopoverDisclosure ref={ref} render={<Slot />}>
-    {children}
-  </Ariakit.PopoverDisclosure>
+ <Ariakit.PopoverDisclosure ref={ref} render={children} />

This also forces you to improve type safety because you'll need to type children as ReactElement, not ReactNode. The latter could cause a runtime error in Slot.

Thanks for the tip, good idea. I hacked the Stackblitz repro pretty quickly, but I'll make sure our actual codebase's types are better.

If you think that this is a separate bug/issue (maybe because of the <Slot>s? but I don't think so), LMK and I'll open another issue on GitHub, but any help on this would be much appreciated.

It indeed appears to be the same problem. Thanks for the reproductions.

About the bug itself, do you happen to have an (even rough) ETA on fixing this? We want to migrate our Popover and Tooltip components to be based on Ariakit, and this is the only blocker for us. Or maybe you could give me some pointers so I can try to fix it myself and send a PR.

[diegohaz]

About the bug itself, do you happen to have an (even rough) ETA on fixing this? We want to migrate our Popover and Tooltip components to be based on Ariakit, and this is the only blocker for us. Or maybe you could give me some pointers so I can try to fix it myself and send a PR.

I can't provide an ETA right now. This kind of issue probably needs more thinking than actual coding.

You don't need to wait, though. If you're still stuck, we should find a workaround that works for you. If your implementation mirrors the last StackBlitz link you posted, then there's no need to use TooltipProvider. Try using component stores instead:

const tooltip = Ariakit.useTooltipStore();

<>
  <Ariakit.Tooltip store={tooltip}>{content}</Ariakit.Tooltip>
  <Ariakit.TooltipAnchor {...props} store={tooltip} render={children} />
</>

[bengry]

You don't need to wait, though. If you're still stuck, we should find a workaround that works for you. If your implementation mirrors the last StackBlitz link you posted, then there's no need to use TooltipProvider. Try using component stores instead:

const tooltip = Ariakit.useTooltipStore();

<>
  <Ariakit.Tooltip store={tooltip}>{content}</Ariakit.Tooltip>
  <Ariakit.TooltipAnchor {...props} store={tooltip} render={children} />
</>

Thanks, our <Tooltip> component is a bit more complex since it both allows a simple content prop as well as using the Compound Components pattern (similar to what I showed for Popover). But I think the underlying issue was the Ariakit Popover provider getting in the way of the tooltip, so changing that to use a self-created store and passing it around using our own context fixed it - basically:

const PopoverScopeContext = React.createContext<{ store: AriakitPopoverStore }>(
  null
);
function usePopoverScope() {
  const value = React.useContext(PopoverScopeContext);
  assert(value != null);
  return value;
}

function PopoverRoot({ open, setOpen, ...rest }: PopoverProps) {
  const store = PopoverPrimitive.usePopoverStore({
    open: props.open,
    setOpen: props.setOpen,
  });

  return (
    <PopoverScopeContext.Provider store={store} {...rest}>
      {rest.children}
    </PopoverScopeContext.Provider>
  );
}

const PopoverTrigger = React.forwardRef(
  ({ asChild, ...props }: PopoverTriggerProps, ref: RefOf<"button">) => {
    const { store } = usePopoverScope();
    return (
      <Ariakit.PopoverDisclosure
        ref={ref}
        store={store}
        render={asChild ? <Slot /> : undefined}
        {...props}
      />
    );
  }
);

// and similarly to `PopoverTrigger` wrapping `<Ariakit.PopoverDisclosure>` and passing in the store explicitly
// the same for `PopoverContent` -> `<Ariakit.Popover>`

@diegohaz Does this make sense as a workaround until this bug is fixed?

[diegohaz]

Yes, that's a perfectly valid use of the lower-level API.

[bengry]

Yes, that's a perfectly valid use of the lower-level API.

I meant the fact that I self-pass the Popover's store rather than the Tooltip's, as you suggested. Why would changing one work while the other wouldn't? (I originally tried doing the Tooltip's, as suggested, but that didn't fix the popover-not-opening issue).

[diegohaz]

Ah, I missed the part where you said the popover doesn't open. The popover isn't opening on the StackBlitz link you shared because you're not spreading the remaining props onto Ariakit.TooltipAnchor. You should do the same for Ariakit.PopoverDisclosure. Custom components must be open for extension. This holds true for both Ariakit's render and Radix's Slot.

As you can see in the snippet I shared:

<Ariakit.TooltipAnchor {...props} store={tooltip} render={children} />
//                      ^^^^^^^^

[bengry]

Ah, I missed the part where you said the popover doesn't open. The popover isn't opening on the StackBlitz link you shared because you're not spreading the remaining props onto Ariakit.TooltipAnchor. You should do the same for Ariakit.PopoverDisclosure. Custom components must be open for extension. This holds true for both Ariakit's render and Radix's Slot.

As you can see in the snippet I shared:

<Ariakit.TooltipAnchor {...props} store={tooltip} render={children} />
//                      ^^^^^^^^

Right, that's a miss on my end in the reproduction. In the app code we do have it spread props of course.
With the changes in place, you still have to either keep an eye on what wraps what in order to get the wanted result. e.g. even when spreading props the tooltip here acts as a popover (opens on click, shows the tooltip's contents):
https://stackblitz.com/edit/vitejs-vite-ygnzwf?file=src%2FApp.tsx

<Popover>
  <Popover.Content>
    Nisi esse consectetur deserunt. Veniam eu irure nulla sint cillum
    proident nisi sunt proident dolor exercitation laboris qui.
  </Popover.Content>

  <Tooltip content="more info"> // A
    <Popover.Trigger> // B
      <Button>i</Button>
    </Popover.Trigger>
  </Tooltip>
</Popover>

Switching up lines A & B fixes it, but tell that to consumers... So I think that this is now the original bug I opened the issue on, correct? Where a Popover & Tooltips mix, such that the Tooltip acts as a Popover, from a user's perspective, and the actual popover content is nowhere to be found.

[diegohaz]

Exactly, that's why I suggested #3754 (comment)