Skip to content

Commit

Permalink
fix(tooltip): fixed tooltip wrapping and changed mouse events to poin…
Browse files Browse the repository at this point in the history
…ter events (#6653)

* fix(tooltip): fixed tooltip wrapping and changed mouse events to pointer events

* fix(tooltip): deprecated the closeOnMouseDown and mapped it with closeOnPointerDown

* fix(tooltip): fixed tests and formatting

* fix(tooltip): prevent opening the tooltip if event is touch
  • Loading branch information
nikolovlazar committed Sep 9, 2022
1 parent 8bdd883 commit 3a79a92
Show file tree
Hide file tree
Showing 7 changed files with 2,418 additions and 660 deletions.
5 changes: 5 additions & 0 deletions .changeset/perfect-cycles-argue.md
@@ -0,0 +1,5 @@
---
"@chakra-ui/tooltip": patch
---

Fixed the wrapping children display + changed mouse events with pointer events
1 change: 1 addition & 0 deletions packages/components/tooltip/package.json
Expand Up @@ -41,6 +41,7 @@
"@chakra-ui/portal": "workspace:*"
},
"devDependencies": {
"@chakra-ui/button": "workspace:*",
"@chakra-ui/shared-utils": "workspace:*",
"@chakra-ui/object-utils": "workspace:*",
"@chakra-ui/hooks": "workspace:*",
Expand Down
6 changes: 5 additions & 1 deletion packages/components/tooltip/src/tooltip.tsx
Expand Up @@ -94,7 +94,11 @@ export const Tooltip = forwardRef<TooltipProps, "div">((props, ref) => {

if (shouldWrap) {
trigger = (
<chakra.span tabIndex={0} {...tooltip.getTriggerProps()}>
<chakra.span
display="inline-block"
tabIndex={0}
{...tooltip.getTriggerProps()}
>
{children}
</chakra.span>
)
Expand Down
30 changes: 19 additions & 11 deletions packages/components/tooltip/src/use-tooltip.ts
Expand Up @@ -38,10 +38,14 @@ export interface UseTooltipProps
*/
closeOnClick?: boolean
/**
* If `true`, the tooltip will hide while the mouse
* is down
* If `true`, the tooltip will hide while the mouse is down
* @deprecated - use `closeOnPointerDown` instead
*/
closeOnMouseDown?: boolean
/**
* If `true`, the tooltip will hide while the pointer is down
*/
closeOnPointerDown?: boolean
/**
* If `true`, the tooltip will hide on pressing Esc key
* @default true
Expand Down Expand Up @@ -87,6 +91,7 @@ export function useTooltip(props: UseTooltipProps = {}) {
closeDelay = 0,
closeOnClick = true,
closeOnMouseDown,
closeOnPointerDown = closeOnMouseDown,
closeOnEsc = true,
onOpen: onOpenProp,
onClose: onCloseProp,
Expand Down Expand Up @@ -165,11 +170,11 @@ export function useTooltip(props: UseTooltipProps = {}) {
}
}, [closeOnClick, closeWithDelay, isOpen])

const onMouseDown = useCallback(() => {
if (isOpen && closeOnMouseDown) {
const onPointerDown = useCallback(() => {
if (isOpen && closeOnPointerDown) {
closeWithDelay()
}
}, [closeOnMouseDown, closeWithDelay, isOpen])
}, [closeOnPointerDown, closeWithDelay, isOpen])

const onKeyDown = useCallback(
(event: KeyboardEvent) => {
Expand All @@ -195,21 +200,24 @@ export function useTooltip(props: UseTooltipProps = {}) {
)

/**
* This allows for catching mouseleave events when the tooltip
* This allows for catching pointerleave events when the tooltip
* trigger is disabled. There's currently a known issue in
* React regarding the onMouseLeave polyfill.
* React regarding the onPointerLeave polyfill.
* @see https://github.com/facebook/react/issues/11972
*/
useEventListener(() => ref.current, "mouseleave", closeWithDelay)
useEventListener(() => ref.current, "pointerleave", closeWithDelay)

const getTriggerProps: PropGetter = useCallback(
(props = {}, _ref = null) => {
const triggerProps = {
...props,
ref: mergeRefs(ref, _ref, referenceRef),
onMouseEnter: callAllHandlers(props.onMouseEnter, openWithDelay),
onPointerEnter: callAllHandlers(props.onPointerEnter, (e) => {
if (e.pointerType === "touch") return
openWithDelay()
}),
onClick: callAllHandlers(props.onClick, onClick),
onMouseDown: callAllHandlers(props.onMouseDown, onMouseDown),
onPointerDown: callAllHandlers(props.onPointerDown, onPointerDown),
onFocus: callAllHandlers(props.onFocus, openWithDelay),
onBlur: callAllHandlers(props.onBlur, closeWithDelay),
"aria-describedby": isOpen ? tooltipId : undefined,
Expand All @@ -220,7 +228,7 @@ export function useTooltip(props: UseTooltipProps = {}) {
[
openWithDelay,
closeWithDelay,
onMouseDown,
onPointerDown,
isOpen,
tooltipId,
onClick,
Expand Down
53 changes: 22 additions & 31 deletions packages/components/tooltip/stories/tooltip.stories.tsx
@@ -1,5 +1,6 @@
import { Modal, ModalContent, ModalOverlay } from "@chakra-ui/modal"
import { Portal } from "@chakra-ui/portal"
import { Button } from "@chakra-ui/button"
import { chakra } from "@chakra-ui/system"
import { AnimatePresence, motion } from "framer-motion"
import * as React from "react"
Expand Down Expand Up @@ -30,7 +31,7 @@ const HookTooltip = ({ children }: any) => {

return (
<>
<button {...getTriggerProps()}>Hover me</button>
<Button {...getTriggerProps()}>Hover me</Button>
<div {...getTooltipPositionerProps()}>
<div
{...getTooltipProps({
Expand Down Expand Up @@ -75,7 +76,7 @@ export const WithTransition = () => {

return (
<>
<button {...getTriggerProps()}>Hover me</button>
<Button {...getTriggerProps()}>Hover me</Button>
<AnimatePresence>
{isOpen && (
<Portal>
Expand Down Expand Up @@ -121,7 +122,7 @@ export const WithTransition = () => {

export const withButton = () => (
<Tooltip label="This is a chakra tooltip" placement="bottom" hasArrow>
<button>Hover me</button>
<Button>Hover me</Button>
</Tooltip>
)

Expand All @@ -137,16 +138,16 @@ export const withAriaLabel = () => (
label="Notifications"
aria-label="3 Notifications"
>
<button style={{ fontSize: 25 }}>
<Button style={{ fontSize: 25 }}>
<span role="img" aria-label="notification">
🔔
</span>
<span>3</span>
</button>
</Button>
</Tooltip>
)

export const issue607 = () => (
export const withinFixedContainer = () => (
<div
style={{
position: "fixed",
Expand All @@ -165,36 +166,36 @@ export const WithModal = () => {
const [showDialog, setShowDialog] = React.useState(false)
return (
<div>
<button onClick={() => setShowDialog(true)}>Show Dialog</button>
<Button onClick={() => setShowDialog(true)}>Show Dialog</Button>
<Modal isOpen={showDialog} onClose={() => setShowDialog(false)}>
<ModalOverlay />
<ModalContent height="300px">
<div>
<button onClick={() => setShowDialog(false)}>Close Dialog</button>
<Button onClick={() => setShowDialog(false)}>Close Dialog</Button>
<Tooltip label="Notifications">
<button style={{ fontSize: 25 }}>
<Button>
<span aria-hidden>🔔</span>
</button>
</Button>
</Tooltip>
<Tooltip label="Settings">
<button style={{ fontSize: 25 }}>
<Button>
<span aria-hidden>⚙️</span>
</button>
</Button>
</Tooltip>
<Tooltip label="Your files are safe with us">
<button style={{ fontSize: 25 }}>
<Button>
<span aria-hidden>💾</span> Save
</button>
</Button>
</Tooltip>

<div style={{ float: "right" }}>
<Tooltip label="Notifications" aria-label="3 Notifications">
<button style={{ fontSize: 25 }}>
<Button>
<span role="img" aria-label="Bell">
🔔
</span>
<span>3</span>
</button>
</Button>
</Tooltip>
</div>
</div>
Expand All @@ -206,40 +207,30 @@ export const WithModal = () => {

export const withDisabledButton = () => (
<Tooltip label="Oh oh oh, oh oh">
<button style={{ fontSize: 25, pointerEvents: "all" }} disabled>
Can't Touch This
</button>
<Button isDisabled>Can't Touch This</Button>
</Tooltip>
)

export const withWrappedDisabledButton = () => (
<Tooltip label="Hello world" shouldWrapChildren>
<button style={{ fontSize: 25, pointerEvents: "all" }} disabled>
Hover me
</button>
<Button isDisabled>Hover me</Button>
</Tooltip>
)

export const withIsOpenProp = () => (
<Tooltip label="Hello world" isOpen hasArrow>
<button style={{ fontSize: 25, pointerEvents: "all" }} disabled>
Can't Touch This
</button>
<Button disabled>Can't Touch This</Button>
</Tooltip>
)

export const withDefaultIsOpenProp = () => (
<Tooltip label="Hello world" defaultIsOpen>
<button style={{ fontSize: 25, pointerEvents: "all" }}>
Can't Touch This
</button>
<Button>Can't Touch This</Button>
</Tooltip>
)

export const withAutoPlacement = () => (
<Tooltip label="Hello world" placement="auto" hasArrow>
<button style={{ fontSize: 25, pointerEvents: "all" }}>
Can't Touch This
</button>
<Button>Can't Touch This</Button>
</Tooltip>
)
36 changes: 18 additions & 18 deletions packages/components/tooltip/tests/tooltip.test.tsx
Expand Up @@ -26,36 +26,36 @@ const DummyComponent = (
test("passes a11y test when hovered", async () => {
render(<DummyComponent />)

fireEvent.mouseOver(screen.getByText(buttonLabel))
fireEvent.pointerOver(screen.getByText(buttonLabel))

const tooltip = await screen.findByRole("tooltip")

await testA11y(tooltip)
})

test("shows on mouseover and closes on mouseleave", async () => {
test("shows on pointerover and closes on pointerleave", async () => {
render(<DummyComponent />)

fireEvent.mouseOver(screen.getByText(buttonLabel))
fireEvent.pointerOver(screen.getByText(buttonLabel))

await screen.findByRole("tooltip")

expect(screen.getByText(buttonLabel)).toBeInTheDocument()
expect(screen.getByRole("tooltip")).toBeInTheDocument()

fireEvent.mouseLeave(screen.getByText(buttonLabel))
fireEvent.pointerLeave(screen.getByText(buttonLabel))

await waitFor(() =>
expect(screen.queryByText(tooltipLabel)).not.toBeInTheDocument(),
)
})

test("should not show on mouseover if isDisabled is true", async () => {
test("should not show on pointerover if isDisabled is true", async () => {
jest.useFakeTimers()

render(<DummyComponent isDisabled />)

fireEvent.mouseOver(screen.getByText(buttonLabel))
fireEvent.pointerOver(screen.getByText(buttonLabel))

act(() => {
jest.advanceTimersByTime(200)
Expand All @@ -66,12 +66,12 @@ test("should not show on mouseover if isDisabled is true", async () => {
jest.useRealTimers()
})

test("should close on mouseleave if openDelay is set", async () => {
test("should close on pointerleave if openDelay is set", async () => {
jest.useFakeTimers()

render(<DummyComponent openDelay={500} />)

fireEvent.mouseOver(screen.getByText(buttonLabel))
fireEvent.pointerOver(screen.getByText(buttonLabel))

act(() => {
jest.advanceTimersByTime(200)
Expand All @@ -83,7 +83,7 @@ test("should close on mouseleave if openDelay is set", async () => {
})
expect(screen.queryByText(tooltipLabel)).toBeInTheDocument()

fireEvent.mouseLeave(screen.getByText(buttonLabel))
fireEvent.pointerLeave(screen.getByText(buttonLabel))

act(() => {
jest.advanceTimersByTime(200)
Expand All @@ -96,37 +96,37 @@ test("should close on mouseleave if openDelay is set", async () => {
jest.useRealTimers()
})

test("should show on mouseover if isDisabled has a falsy value", async () => {
test("should show on pointerover if isDisabled has a falsy value", async () => {
render(<DummyComponent isDisabled={false} />)

fireEvent.mouseOver(screen.getByText(buttonLabel))
fireEvent.pointerOver(screen.getByText(buttonLabel))

await screen.findByRole("tooltip")

expect(screen.getByText(buttonLabel)).toBeInTheDocument()
})

test("should close on mouseleave if shouldWrapChildren is true and child is a disabled element", async () => {
test("should close on pointerleave if shouldWrapChildren is true and child is a disabled element", async () => {
render(<DummyComponent shouldWrapChildren isButtonDisabled />)

fireEvent.mouseEnter(screen.getByText(buttonLabel))
fireEvent.pointerEnter(screen.getByText(buttonLabel))

await screen.findByRole("tooltip")

const wrapper = screen.getByText(buttonLabel).parentElement
expect(wrapper).not.toBeNull()

fireEvent.mouseLeave(wrapper!)
fireEvent.pointerLeave(wrapper!)

await waitFor(() =>
expect(screen.queryByText(tooltipLabel)).not.toBeInTheDocument(),
)
})

test("shows on mouseover and closes on pressing 'esc'", async () => {
test("shows on pointerover and closes on pressing 'esc'", async () => {
const { user } = render(<DummyComponent />)

fireEvent.mouseOver(screen.getByText(buttonLabel))
fireEvent.pointerOver(screen.getByText(buttonLabel))

await screen.findByRole("tooltip")

Expand All @@ -140,10 +140,10 @@ test("shows on mouseover and closes on pressing 'esc'", async () => {
)
})

test("shows on mouseover and stays on pressing 'esc' if 'closeOnEsc' is false", async () => {
test("shows on pointerover and stays on pressing 'esc' if 'closeOnEsc' is false", async () => {
const { user } = render(<DummyComponent closeOnEsc={false} />)

fireEvent.mouseOver(screen.getByText(buttonLabel))
fireEvent.pointerOver(screen.getByText(buttonLabel))

await screen.findByRole("tooltip")

Expand Down

1 comment on commit 3a79a92

@vercel
Copy link

@vercel vercel bot commented on 3a79a92 Sep 9, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.