adobe / react-spectrum

A collection of libraries and tools that help you build adaptive, accessible, and robust user experiences.
https://react-spectrum.adobe.com
Apache License 2.0
12.2k stars 1.07k forks source link

Tooltip not working with hoverable and focusable components other than React Aria Buttons #6142

Open RexusWolf opened 3 months ago

RexusWolf commented 3 months ago

Provide a general summary of the issue here

We are trying to use the Tooltip to create Tags that, when hovered, show a message.

Specifically, we have Tags (TagGroup) with some text and a Remove Button.

If we remove the Button from the Tag, the Tooltip never appears, even if it's a controlled open state. This makes it impossible to use Tooltips with components that are not buttons. And specifically, react-aria-components Buttons. It doesn't work with pure HTML buttons.

Additionally, we want to place the Tooltip on top of the Tag, pointing to the exact center of the tag. However, the Tooltip is placed in reference to the Remove button of the tag instead. We can reposition it using the triggerRef, but we wonder if it's because of the first behavior that this happens.

We would appreciate any help :)

Thank you.

P.S: We are aware that interactions on hover aren't very recommended, but it's required in our use case.

๐Ÿค” Expected Behavior?

The tooltip should be usable with any component that is hoverable and focusable, and it should be placed using the whole content of the Tooltip as a reference.

๐Ÿ˜ฏ Current Behavior

The tooltip should does not work with any component that is hoverable and focusable, other than React Aria Button, and it is placed using the first Button inside the tooltip as a reference.

๐Ÿ’ Possible Solution

No response

๐Ÿ”ฆ Context

We want the Tooltip to be usable with any hoverable and focusable component, like Tags, Buttons (our own buttons), etc.

๐Ÿ–ฅ๏ธ Steps to Reproduce

  1. Create a component that uses TooltipTrigger, Tooltip and has a button as trigger.
  2. Hover over the button.
  3. The tooltip should appear but it doesn't.
import {Button, OverlayArrow, Tooltip, TooltipTrigger} from 'react-aria-components';

<TooltipTrigger>
  <button>โœ๏ธ</button>
  <Tooltip>
    <OverlayArrow>
      <svg width={8} height={8} viewBox="0 0 8 8">
        <path d="M0 0 L4 4 L8 0" />
      </svg>
    </OverlayArrow>
    Edit
  </Tooltip>
</TooltipTrigger>

Version

react-aria-components 1.1.1

What browsers are you seeing the problem on?

Chrome

If other, please specify.

No response

What operating system are you using?

MacOS

๐Ÿงข Your Company/Team

No response

๐Ÿ•ท Tracking Issue

No response

reidbarber commented 3 months ago

The Tooltip should work with any component using useHover and useFocusable. So this could be a button created with useButton, and doesn't necessarily need to be a RAC Button. This is because it doesn't rely on native focus/hover state, but instead on the unified behavior provided by those hooks.

I think the tooltip not working with Tag is a different question, and may be related to how collections work. That might need more investigation. My initial thought it is that this isn't working due to useSelectableItem not calling useFocusable, or due to some implementation detail of our RAC collections.

Niznikr commented 3 months ago

Yeah it would be helpful to be able to wrap TooltipTrigger around Tag, MenuItem, etc when working with RAC collections.

snowystinger commented 3 months ago

Unfortunately, Tooltips will not be added around MenuItem. For that we recommend a pattern such as https://react-spectrum.adobe.com/react-spectrum/Menu.html#unavailable-items or including a description text. Tooltips are not accessible and should not be used to convey critical information.

Here are some good usage guidelines around using tooltips https://spectrum.adobe.com/page/tooltip/#Usage-guidelines

damianstasik commented 2 months ago

@snowystinger any chance of adding primitives to build something like ContextualHelpTrigger with RAC? There's a bit of complexity due to how the help dialog is shown using a submenu trigger, so it would be awesome to have a dedicated component/hook that would take care of that.

snowystinger commented 2 months ago

We have a plan to eventually add something we're referring to as "sub dialogs" as opposed to "sub menus", which is what could be used to create this more easily. I know we weren't entirely happy with the API we have in RSP right now for it. Let me check with the team and see where it is in our priorities.

unional commented 1 month ago

The Tooltip should work with any component using useHover and useFocusable

Do you mean useFocusableRef()? I can't find useFocusable and also don't know how to make it work for useHover.

Is there an example I can look at?

snowystinger commented 1 month ago

here's where useButton uses useFocusable https://github.com/adobe/react-spectrum/blob/0c17289de18041e6f8b99df26a5a3ca922cc5145/packages/%40react-aria/button/src/useButton.ts#L99

RAC will pass some stuff on a context that that hook uses. If you're using the hooks, you'll have to handle that yourself. https://github.com/adobe/react-spectrum/blob/0c17289de18041e6f8b99df26a5a3ca922cc5145/packages/react-aria-components/src/Tooltip.tsx#L89

useTooltipTrigger will also create onHover listeners, these should be passed through useHover, see RAC implementation for how that works

unional commented 3 weeks ago

When I use it on a focusable component, the placement is incorrect:

image

And when I try to repro it on codesandbox, it doesn't work:

sandbox link

I'm using the same versions locally and in codesandbox:

snowystinger commented 6 days ago

Ah, you've just missed making a ref. Here, I've fixed up the sandbox https://codesandbox.io/p/sandbox/beautiful-brook-pkg7yc?file=%2Fsrc%2FDemo.jsx%3A7%2C32

unional commented 6 days ago

@snowystinger thanks, that works!

A related question to useFocusable, I'm getting type error:

const Focusable = forwardRef<HTMLSpanElement, PropsWithChildren<FocusableOptions>>((props, forwardedRef) => {
    // forwardRef doesn't guarantee a ref, so need to create a backup one so that we have a ref to our trigger every time
    const backupRef = useRef(null)
    const ref = forwardedRef ?? backupRef
        // Argument of type '((instance: HTMLSpanElement | null) => void) | MutableRefObject<HTMLSpanElement | null>' is not assignable to parameter of type 'RefObject<FocusableElement>'.
        // Type '(instance: HTMLSpanElement | null) => void' is not assignable to type 'RefObject<FocusableElement>'
    const { focusableProps } = useFocusable(props, ref)
    return <span {...props} {...focusableProps} ref={ref} />
})

In react-spectrum code, it uses useFocusableRef instead. Should I create something similar and use that instead?