From 5445d5e9f44d96ee4adfbe1c751161421d7b7fc1 Mon Sep 17 00:00:00 2001 From: Ryan Seddon Date: Thu, 1 Aug 2019 15:05:54 +1000 Subject: [PATCH] fix(tooltip): don't pass in a ref as it's not actually needed --- packages/tooltip/README.md | 14 +++---------- packages/tooltip/src/TooltipContainer.js | 1 - packages/tooltip/src/TooltipContainer.spec.js | 9 +++------ packages/tooltip/src/useTooltip.js | 20 +++++++++++++++---- 4 files changed, 22 insertions(+), 22 deletions(-) diff --git a/packages/tooltip/README.md b/packages/tooltip/README.md index 54bf95afe..0c693e168 100644 --- a/packages/tooltip/README.md +++ b/packages/tooltip/README.md @@ -16,14 +16,10 @@ For live examples check out our [storybook](https://zendeskgarden.github.io/reac ### useTooltip ```jsx static -import { useRef } from 'react'; import { useTooltip } from '@zendeskgarden/container-tooltip'; const Tooltip = () => { - const tooltipRef = useRef(null); - const { isVisible, getTooltipProps, getTriggerProps } = useTooltip({ - tooltipRef, isVisible: false, delayMilliseconds: 500 }); @@ -38,8 +34,8 @@ const Tooltip = () => { return ( <> -
Tooltip
- +
Tooltip
+ ); }; @@ -48,14 +44,11 @@ const Tooltip = () => { ### TooltipContainer ```jsx static -import { useRef } from 'react'; import { TooltipContainer } from '@zendeskgarden/container-tooltip'; const Tooltip = () => { - const tooltipRef = useRef(null); - return ( - + {({ isVisible, getTooltipProps, getTriggerProps }) => { const styles = { visibility: isVisible ? 'visible' : 'hidden', @@ -69,7 +62,6 @@ const Tooltip = () => { <>
diff --git a/packages/tooltip/src/TooltipContainer.js b/packages/tooltip/src/TooltipContainer.js index f415edfa3..da847b708 100644 --- a/packages/tooltip/src/TooltipContainer.js +++ b/packages/tooltip/src/TooltipContainer.js @@ -16,7 +16,6 @@ export function TooltipContainer({ children, render = children, ...options }) { TooltipContainer.propTypes = { children: PropTypes.func, render: PropTypes.func, - tooltipRef: PropTypes.shape({ current: PropTypes.instanceOf(Element) }).isRequired, delayMilliseconds: PropTypes.number, isVisible: PropTypes.bool }; diff --git a/packages/tooltip/src/TooltipContainer.spec.js b/packages/tooltip/src/TooltipContainer.spec.js index f1371bfe7..eb9761b96 100644 --- a/packages/tooltip/src/TooltipContainer.spec.js +++ b/packages/tooltip/src/TooltipContainer.spec.js @@ -5,7 +5,7 @@ * found at http://www.apache.org/licenses/LICENSE-2.0. */ -import React, { useRef } from 'react'; +import React from 'react'; import { render, fireEvent } from '@testing-library/react'; import { act } from 'react-dom/test-utils'; @@ -17,17 +17,14 @@ describe('TooltipContainer', () => { const TOOLTIP_ID = 'test'; const BasicExample = props => { - const tooltipRef = useRef(null); - return ( - + {({ getTooltipProps, getTriggerProps }) => ( <>
trigger
tooltip diff --git a/packages/tooltip/src/useTooltip.js b/packages/tooltip/src/useTooltip.js index 7e4a1fa8c..0e43d5f9b 100644 --- a/packages/tooltip/src/useTooltip.js +++ b/packages/tooltip/src/useTooltip.js @@ -5,12 +5,13 @@ * found at http://www.apache.org/licenses/LICENSE-2.0. */ -import { useState, useEffect } from 'react'; +import { useState, useEffect, useRef } from 'react'; import { composeEventHandlers, generateId, KEY_CODES } from '@zendeskgarden/container-utilities'; -export function useTooltip({ tooltipRef, delayMilliseconds = 500, id, isVisible } = {}) { +export function useTooltip({ delayMilliseconds = 500, id, isVisible } = {}) { const [visibility, setVisibility] = useState(isVisible); const [_id] = useState(id || generateId('garden-tooltip-container')); + const isMounted = useRef(false); let openTooltipTimeout; let closeTooltipTimeout; @@ -19,7 +20,7 @@ export function useTooltip({ tooltipRef, delayMilliseconds = 500, id, isVisible clearTimeout(closeTooltipTimeout); openTooltipTimeout = setTimeout(() => { - if (tooltipRef.current) { + if (isMounted.current) { setVisibility(true); } }, delayMs); @@ -29,12 +30,23 @@ export function useTooltip({ tooltipRef, delayMilliseconds = 500, id, isVisible clearTimeout(openTooltipTimeout); closeTooltipTimeout = setTimeout(() => { - if (tooltipRef.current) { + if (isMounted.current) { setVisibility(false); } }, delayMs); }; + // Sometimes the timeout will call setVisibility even after unmount and cleanup + // reproducable when running tests, happens when fast switching in storybook. + // May be related https://github.com/facebook/react/pull/15650 + useEffect(() => { + isMounted.current = true; + + return () => { + isMounted.current = false; + }; + }, []); + // Clean up stray timeouts if tooltip unmounts useEffect(() => { return () => {