radix-ui / primitives

Radix Primitives is an open-source UI component library for building high-quality, accessible design systems and web apps. Maintained by @workos.
https://radix-ui.com/primitives
MIT License
15.68k stars 810 forks source link

[Tooltip] Tooltip component doesn't work on iOS Safari (14.4) #955

Closed jordie23 closed 2 years ago

jordie23 commented 2 years ago

Bug report

The Tooltip component isn't working on iOS Safari.

Current Behavior

Tapping on a tooltip component doesn't show the content

Expected behavior

Content shows when tapping a Tooltip component

Reproducible example

  1. Load up this page on iOS (14.4 is what I used in simulator and 14.6 on my own phone):

https://www.radix-ui.com/docs/primitives/components/tooltip

  1. Tap the + in the example.

Suggested solution

Content should show on tap events

Your environment

Below is my env, but it's also not working on whatever your website env is.

Software Name(s) Version
Radix Package(s) @radix-ui/react-tooltip 0.1.1
React n/a 17.0.2
Browser Safari iOS 14.4 ad 14.6
Assistive tech n/a
Node n/a v14.17.3
npm/yarn npm 6.14.13
Operating System iOS 14.4
benoitgrelard commented 2 years ago

This is by design. Tooltips are problematic on touch devices because there is no hover interaction, so if it was on tap it would fight with the general button action, or require 2 taps. That's also why tooltips should just be used generally for extra information that isn't mandatory do use your interface.

jjenzz commented 2 years ago

I wonder if it is worth looking into this though. Material UI provide a "touch" option for this which appears to be a longpress: https://mui.com/components/tooltips/#triggers

Perhaps we can do something similar?

jordie23 commented 2 years ago

Tooltips aren't usually just superfluous information, they generally important information just hidden away until required. I do typically see the hover ability on desktop replaced with a tap action on mobile.

I mean, here, on your own documentation page you've made the tooltips accessible on mobile devices. Why would you do that if it wasn't important information?

IMG_6009

I'd suggest one or more of the following actions:

benoitgrelard commented 2 years ago

I wonder if it is worth looking into this though. Material UI provide a "touch" option for this which appears to be a longpress: https://mui.com/components/tooltips/#triggers Perhaps we can do something similar?

I personally don't think that makes much sense (for the reasons highlighted below). A tooltip is usually used to provide more info about a trigger that already does something. How would you know you can long press it?

I mean, here, on your own documentation page you've made the tooltips accessible on mobile devices. Why would you do that if it wasn't important information?

That is not a tooltip, that's a popover, the interaction for those is pointer down. The big difference here is that there isn't 2 functionality attached to the same trigger. Tooltips work fine with focus or hover, because they are attached to triggers with pointer down actions.

So it looks like perhaps we need to emphasize more in the docs the differences between the patterns?

jordie23 commented 2 years ago

That is not a tooltip, that's a popover, the interaction for those is pointer down. The big difference here is that there isn't 2 functionality attached to the same trigger. Tooltips work fine with focus or hover, because they are attached to triggers with pointer down actions.

So it looks like perhaps we need to emphasize more in the docs the differences between the patterns?

AHHHH I see. That makes so much more sense now. I guess I didn't think about the popover being used in that way, I figured it was always for more complex use cases. Thanks for the clarification.

I think perhaps a note on the Tooltip page referencing the Popover might help silly people like me who generalise what a Tooltip is?

jjenzz commented 2 years ago

A tooltip is usually used to provide more info about a trigger that already does something

True. Plus, I just realised that the WAI ARIA tooltip docs (wip) suggest they're intended for focus/hover only.

I think perhaps a note on the Tooltip page referencing the Popover might help silly people like me who generalise what a Tooltip is?

Agreed. We have spoken about having a "When to use" section for the components as these patterns can appear to overlap, it's all a bit confusing. I'll let the team know that we've had another request for this :+1:

EDIT: I created a new issue on the website repo instead https://github.com/radix-ui/website/issues/272

yangmingshan commented 2 years ago

I found a naive way to make Tooltip working on both pointing device and touch device. In my use case, it's just an info icon like your website, it's not a general button, so there will be no event conflict. Not sure any downside of this solution.

https://codesandbox.io/s/smoosh-forest-553n1q?file=/App.js

And it let me thinking, can we add an option to make Tooltip working on both pointing device and touch device, but it can not be a general button, so we can avoid event conflict? 🤔

guidominguesnk commented 2 years ago

You are a god

yangmingshan commented 2 years ago

@benoitgrelard Sorry to ping you, but what's your thoughts?

benoitgrelard commented 2 years ago

Hey @yangmingshan,

In my use case, it's just an info icon like your website, it's not a general button, so there will be no event conflict. Not sure any downside of this solution.

In that case, a Popover is probably more appropriate then.

And it let me thinking, can we add an option to make Tooltip working on both pointing device and touch device, but it can not be a general button, so we can avoid event conflict? 🤔

No that's not possible, a tooltip is here to display a "tip" regarding a given "tool", there should always be an action behind this, so it's not possible to avoid event conflicts.

kaptn3 commented 1 year ago

@benoitgrelard hi! sorry for ping, but what else I need opens popover on hover event on desktop and on click event on mobile devices, what I must use popover or tooltip?

benoitgrelard commented 1 year ago

@kaptn3 If you want an interaction that works on mobile (touch) then it would have to be popover. That said, it doesn't open on hover on desktop as that's not the intended behaviour of a popover, so you'd have to override that behaviour if that's what you wanted, I'd warn you against it though as that's not really natural as focus moves into the popover when it opens for accessibility reasons.

kaptn3 commented 1 year ago

@benoitgrelard got it, thank you!

sksiyer commented 1 year ago

@benoitgrelard I think that the convention for mobile is that tooltips appear... See Bootstrap: https://getbootstrap.com/docs/4.0/components/tooltips See Carbon: https://carbondesignsystem.com/components/tooltip/code

benoitgrelard commented 1 year ago

There are no such convention in the existing accessibility guidelines as touch is problematic.

This is because a tooltip is a "tip" for a "tool", that tool is an interactive element (99% of the time should be a button), hence on touch devices if you'd show the tooltip on touch, it would already be too late anyway because the action the tool performs would have been triggered so it simply doesn't make much sense. Other alternatives result in poor UX such as asking the user to press twice (once for the tooltip, and a second for the action).

Hopefully this makes sense why they are ignored on touch.

See here where it specifically only mentions hover and focus: https://www.w3.org/WAI/ARIA/apg/patterns/tooltip/

MaximilianDietel03 commented 1 year ago

I've came across this issue as well and ended up modifying the Popover component to fit my requirements. When using the exported components, the Popover is touchable and hoverable.

import * as React from 'react'
import { FC, useContext, useState } from 'react'

import * as PopoverPrimitive from '@radix-ui/react-popover'

const PopoverOnOpenChangeContext = React.createContext({
  onOpenChange: (open: boolean) => {
    /* empty */
  },
  timeoutId: { current: null } as React.MutableRefObject<ReturnType<typeof setTimeout> | null>,
})

const Popover: FC<React.ComponentProps<typeof PopoverPrimitive.Root>> = ({
  open,
  onOpenChange,
  ...props
}) => {
  const [defaultOpen, defaultOnOpenChange] = useState(false)
  const timeoutId = React.useRef(null)

  return (
    <PopoverOnOpenChangeContext.Provider
      value={{
        onOpenChange: onOpenChange ?? defaultOnOpenChange,
        timeoutId,
      }}
    >
      <PopoverPrimitive.Root
        {...props}
        open={open ?? defaultOpen}
        onOpenChange={onOpenChange ?? defaultOnOpenChange}
      />
    </PopoverOnOpenChangeContext.Provider>
  )
}

const PopoverTrigger: React.FC<any> = (props) => {
  const { onOpenChange, timeoutId } = useContext(PopoverOnOpenChangeContext)

  return (
    <PopoverPrimitive.Trigger
      {...props}
      onMouseEnter={() => {
        timeoutId.current && clearTimeout(timeoutId.current)
        onOpenChange(true)
      }}
      onMouseLeave={() => {
        const _timeoutId = setTimeout(() => onOpenChange(false), 300)
        timeoutId.current = _timeoutId
      }}
    />
  )
}

const PopoverContent = React.forwardRef<
  React.ElementRef<typeof PopoverPrimitive.Content>,
  React.PropsWithoutRef<React.ComponentProps<typeof PopoverPrimitive.Content>>
>(({ align = 'center', sideOffset = 4, ...props }, ref) => {
  const { onOpenChange, timeoutId } = useContext(PopoverOnOpenChangeContext)

  return (
    <PopoverPrimitive.Portal>
      <PopoverPrimitive.Content
        ref={ref}
        align={align}
        sideOffset={sideOffset}
        onMouseEnter={() => {
          timeoutId.current && clearTimeout(timeoutId.current)
        }}
        onMouseLeave={() => {
          const _timeoutId = setTimeout(() => onOpenChange(false), 300)
          timeoutId.current = _timeoutId
        }}
        {...props}
      />
    </PopoverPrimitive.Portal>
  )
})
PopoverContent.displayName = PopoverPrimitive.Content.displayName

export { Popover, PopoverContent, PopoverTrigger }
matstyler commented 1 year ago

Simple implementation for both - mobile and desktop:

export default function Tooltip({ alwaysOpen, children }: Props): ReactElement {
    const [open, setOpen] = useState(false)

    return (
        <RadixTooltip.Root
            open={alwaysOpen || open}
            delayDuration={0}
            onOpenChange={setOpen}
        >
            <div onClick={() => setOpen(true)}>{children}</div>
        </RadixTooltip.Root>
    )
}
kainanaina commented 11 months ago

To add to solution above, there was still an issue where on mobile I needed to click twice to get the tooltip, because onOpenChange was running before click handler on initial click somehow. The "solution" was to add timeout state update to onFocus on Trigger element.

Here is the final code:

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

  return (
    <RadixTooltip.Provider>
      <RadixTooltip.Root open={open} onOpenChange={setOpen}>
        <RadixTooltip.Trigger
          onClick={() => setOpen((prevOpen) => !prevOpen)}
          onFocus={() => setTimeout(() => setOpen(true), 0)} // timeout needed to run this after onOpenChange to prevent bug on mobile
          onBlur={() => setOpen(false)}
        >
          {children}
        </RadixTooltip.Trigger>
        <RadixTooltip.Portal>
          <RadixTooltip.Content>
            // tooltip content
            <RadixTooltip.Arrow />
          </RadixTooltip.Content>
        </RadixTooltip.Portal>
      </RadixTooltip.Root>
    </RadixTooltip.Provider>
  );
neos04 commented 2 months ago

I found a naive way to make Tooltip working on both pointing device and touch device. In my use case, it's just an info icon like your website, it's not a general button, so there will be no event conflict. Not sure any downside of this solution.

https://codesandbox.io/s/smoosh-forest-553n1q?file=/App.js

And it let me thinking, can we add an option to make Tooltip working on both pointing device and touch device, but it can not be a general button, so we can avoid event conflict? 🤔

How would this work for multiple tooltips tho?

Geczy commented 2 months ago

for those using shadcn:

"use client";

import * as TooltipPrimitive from "@radix-ui/react-tooltip";
import * as React from "react";

import { cn } from "@/lib/utils";

const TooltipProvider = TooltipPrimitive.Provider;

const Tooltip = ({
    alwaysOpen,
    children,
}: {
    alwaysOpen?: boolean;
    children: React.ReactNode;
}) => {
    const [open, setOpen] = React.useState(false);

    return (
        <TooltipPrimitive.Root open={alwaysOpen || open} onOpenChange={setOpen}>
            <div
                onClick={(e) => {
                    setOpen(true);
                }}
                onKeyUp={(e) => {
                    if (e.key === "Enter" || e.key === " ") {
                        setOpen(true);
                    }
                }}
                onKeyDown={(e) => {
                    if (e.key === "Enter" || e.key === " ") {
                        setOpen(true);
                    }
                }}
            >
                {children}
            </div>
        </TooltipPrimitive.Root>
    );
};

const TooltipTrigger = ({
    onClick,
    onFocus,
    onBlur,
    onKeyUp,
    onKeyDown,
    children,
}: {
    onClick?: React.MouseEventHandler<HTMLDivElement>;
    onFocus?: React.FocusEventHandler<HTMLDivElement>;
    onBlur?: React.FocusEventHandler<HTMLDivElement>;
    onKeyUp?: React.KeyboardEventHandler<HTMLDivElement>;
    onKeyDown?: React.KeyboardEventHandler<HTMLDivElement>;
    children: React.ReactNode;
}) => {
    const [open, setOpen] = React.useState(false);

    const handleKeyEvents = (event: React.KeyboardEvent<HTMLDivElement>) => {
        if (onKeyUp) onKeyUp(event);
        if (onKeyDown) onKeyDown(event);
        if (event.key === "Enter" || event.key === " ") {
            setOpen((prevOpen) => !prevOpen);
        }
    };

    return (
        <TooltipPrimitive.Trigger asChild>
            <div
                tabIndex={0}
                role="button"
                onClick={(e) => {
                    setOpen((prevOpen) => !prevOpen);
                    if (onClick) onClick(e);
                }}
                onFocus={(e) => {
                    setTimeout(() => setOpen(true), 0);
                    if (onFocus) onFocus(e);
                }}
                onBlur={(e) => {
                    setOpen(false);
                    if (onBlur) onBlur(e);
                }}
                onKeyUp={handleKeyEvents}
                onKeyDown={handleKeyEvents}
            >
                {children}
            </div>
        </TooltipPrimitive.Trigger>
    );
};

const TooltipContent = React.forwardRef<
    React.ElementRef<typeof TooltipPrimitive.Content>,
    React.ComponentPropsWithoutRef<typeof TooltipPrimitive.Content>
>(({ className, sideOffset = 4, ...props }, ref) => (
    <TooltipPrimitive.Content
        ref={ref}
        sideOffset={sideOffset}
        className={cn(
            "z-50 overflow-hidden rounded-md border bg-popover px-3 py-1.5 text-sm text-popover-foreground shadow-md animate-in fade-in-0 zoom-in-95 data-[state=closed]:animate-out data-[state=closed]:fade-out-0 data-[state=closed]:zoom-out-95 data-[side=bottom]:slide-in-from-top-2 data-[side=left]:slide-in-from-right-2 data-[side=right]:slide-in-from-left-2 data-[side=top]:slide-in-from-bottom-2",
            className,
        )}
        {...props}
    />
));
TooltipContent.displayName = TooltipPrimitive.Content.displayName;

export { Tooltip, TooltipTrigger, TooltipContent, TooltipProvider };
coreyward commented 4 weeks ago

@benoitgrelard I feel like the stance you're arguing from on this is a bit strained and silly. Of course the original tooltip was not considering touch devices—it was invented in the 90s. UI design has evolved a lot since then, as have input modalities.

While I think it's reasonable for someone to have a tooltip on a button intending to be clicked, that's not the most common use case for them today. Rather than pretending you'd have to boil the ocean (serve every single use case ever), just add the expected behavior to show the tooltip on touch-start and hide it when the user clicks outside as the default. It (still) won't work when someone has an interactive button, but that's fine—at least it'll work in the majority of use cases that have produced a dozen clumsy workarounds (and likely sent plenty of potential Radix UI users searching for another library).

Pragmatism > Perfectionism