Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix(tooltip): don't pass in a ref as it's not actually needed #82

Merged
merged 1 commit into from Aug 2, 2019
Merged

fix(tooltip): don't pass in a ref as it's not actually needed #82

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
14 changes: 3 additions & 11 deletions packages/tooltip/README.md
View file
Open in desktop
Expand Up @@ -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
});
Expand All @@ -38,8 +34,8 @@ const Tooltip = () => {

return (
<>
<div {...getTooltipProps({ ref: tooltipRef, style: styles })}>Tooltip</div>
<button {...getTriggerProps({ ref: triggerRef })}>Trigger</button>
<div {...getTooltipProps({ style: styles })}>Tooltip</div>
<button {...getTriggerProps()}>Trigger</button>
</>
);
};
Expand All @@ -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 (
<TooltipContainer tooltipRef={tooltipRef} isVisible={false} delayMilliseconds={500}>
<TooltipContainer isVisible={false} delayMilliseconds={500}>
{({ isVisible, getTooltipProps, getTriggerProps }) => {
const styles = {
visibility: isVisible ? 'visible' : 'hidden',
Expand All @@ -69,7 +62,6 @@ const Tooltip = () => {
<>
<div
{...getTooltipProps({
ref: tooltipRef,
style: styles
})}
>
Expand Down
1 change: 0 additions & 1 deletion packages/tooltip/src/TooltipContainer.js
View file
Open in desktop
Expand Up @@ -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
};
9 changes: 3 additions & 6 deletions packages/tooltip/src/TooltipContainer.spec.js
View file
Open in desktop
Expand Up @@ -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';

Expand All @@ -17,17 +17,14 @@ describe('TooltipContainer', () => {
const TOOLTIP_ID = 'test';

const BasicExample = props => {
const tooltipRef = useRef(null);

return (
<TooltipContainer tooltipRef={tooltipRef} id={TOOLTIP_ID} {...props}>
<TooltipContainer id={TOOLTIP_ID} {...props}>
{({ getTooltipProps, getTriggerProps }) => (
<>
<div {...getTriggerProps({ 'data-test-id': 'trigger' })}>trigger</div>
<div
{...getTooltipProps({
'data-test-id': 'tooltip',
ref: tooltipRef
'data-test-id': 'tooltip'
})}
>
tooltip
Expand Down
20 changes: 16 additions & 4 deletions packages/tooltip/src/useTooltip.js
View file
Open in desktop
Expand Up @@ -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;
Expand All @@ -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);
Expand All @@ -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 () => {
Expand Down