From 139c025e33edcaafddb8a0ba6cc7fc9b0eb51165 Mon Sep 17 00:00:00 2001 From: Aaron Casanova <32409546+aaronccasanova@users.noreply.github.com> Date: Fri, 15 Mar 2024 17:37:51 -0700 Subject: [PATCH] Revert "Add controlled prop to open the Tooltip" (#11751) Reverts Shopify/polaris#11714 @chloerice and I are exploring an alternative pattern for hybrid controlled/uncontrolled component state management. --- .changeset/three-plants-smile.md | 5 - .../components/Tooltip/Tooltip.stories.tsx | 86 ++----------- .../src/components/Tooltip/Tooltip.tsx | 26 ++-- .../components/Tooltip/tests/Tooltip.test.tsx | 118 ++---------------- .../pages/examples/tooltip-default.tsx | 2 +- .../tooltip-with-persistence-on-click.tsx | 6 +- .../pages/examples/tooltip-with-underline.tsx | 6 +- 7 files changed, 37 insertions(+), 212 deletions(-) delete mode 100644 .changeset/three-plants-smile.md diff --git a/.changeset/three-plants-smile.md b/.changeset/three-plants-smile.md deleted file mode 100644 index 3dfafa6c163..00000000000 --- a/.changeset/three-plants-smile.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -'@shopify/polaris': minor ---- - -Added the `open` prop to `Tooltip` to support programmatic control of its active state. Added the `defaultOpen` prop to `Tooltip` to supersede the `active` prop for setting the initial active state. Deprecated the `active` prop on `Tooltip` to be removed in Polaris v13. diff --git a/polaris-react/src/components/Tooltip/Tooltip.stories.tsx b/polaris-react/src/components/Tooltip/Tooltip.stories.tsx index 4869f0b286d..173bf0a2fc3 100644 --- a/polaris-react/src/components/Tooltip/Tooltip.stories.tsx +++ b/polaris-react/src/components/Tooltip/Tooltip.stories.tsx @@ -1,4 +1,4 @@ -import React, {useCallback, useState} from 'react'; +import React, {useState} from 'react'; import {QuestionCircleIcon} from '@shopify/polaris-icons'; import type {ComponentMeta} from '@storybook/react'; import { @@ -43,7 +43,7 @@ export function All() { export function Default() { return ( - + Order #1001 @@ -57,7 +57,7 @@ export function PreferredPosition() { @@ -75,7 +75,7 @@ export function PreferredPosition() { @@ -102,7 +102,7 @@ export function Width() { @@ -119,7 +119,7 @@ export function Width() { @@ -145,7 +145,7 @@ export function Padding() { return ( - + Tooltip with @@ -160,7 +160,7 @@ export function Padding() { @@ -187,7 +187,7 @@ export function BorderRadius() { @@ -204,7 +204,7 @@ export function BorderRadius() { @@ -307,7 +307,7 @@ export function ActivatorAsDiv() { return ( @@ -462,7 +462,7 @@ export function Alignment() { export function HasUnderline() { return ( - + Order #1001 @@ -486,66 +486,6 @@ export function PersistOnClick() { ); } -export function WithControlledState() { - const [open, setOpen] = useState(false); - - const handleOpen = useCallback(() => { - setOpen(true); - }, []); - - const handleClose = useCallback(() => { - setOpen(false); - }, []); - - return ( - - - - The tooltip is {String(open)} - - - - ); -} - -export function WithUncontrolledState() { - return ( - - - - - Default open true - - - - - Default open false - - - - - Default open undefined - - - - - ); -} - export function ActiveStates() { const [popoverActive, setPopoverActive] = useState(false); const [tooltipActive, setTooltipActive] = @@ -618,7 +558,7 @@ export function ActiveStates() { export function OneCharacter() { return ( - + Order #1001 diff --git a/polaris-react/src/components/Tooltip/Tooltip.tsx b/polaris-react/src/components/Tooltip/Tooltip.tsx index 0c693ce0a4c..635e3977fed 100644 --- a/polaris-react/src/components/Tooltip/Tooltip.tsx +++ b/polaris-react/src/components/Tooltip/Tooltip.tsx @@ -23,14 +23,7 @@ export interface TooltipProps { children?: React.ReactNode; /** The content to display within the tooltip */ content: React.ReactNode; - /** Toggle whether the tooltip is visible. */ - open?: boolean; - /** Toggle whether the tooltip is visible initially */ - defaultOpen?: boolean; - /** - * Toggle whether the tooltip is visible initially - * @deprecated Use `defaultOpen` instead - */ + /** Toggle whether the tooltip is visible */ active?: boolean; /** Delay in milliseconds while hovering over an element before the tooltip is visible */ hoverDelay?: number; @@ -81,8 +74,6 @@ export function Tooltip({ children, content, dismissOnMouseOut, - open, - defaultOpen: defaultOpenProp, active: originalActive, hoverDelay, preferredPosition = 'above', @@ -100,15 +91,14 @@ export function Tooltip({ const borderRadius = borderRadiusProp || '200'; const WrapperComponent: any = activatorWrapper; - const defaultOpen = defaultOpenProp ?? originalActive; const { value: active, setTrue: setActiveTrue, setFalse: handleBlur, - } = useToggle(Boolean(defaultOpen)); + } = useToggle(Boolean(originalActive)); const {value: persist, toggle: togglePersisting} = useToggle( - Boolean(defaultOpen) && Boolean(persistOnClick), + Boolean(originalActive) && Boolean(persistOnClick), ); const [activatorNode, setActivatorNode] = useState(null); @@ -118,14 +108,14 @@ export function Tooltip({ const id = useId(); const activatorContainer = useRef(null); const mouseEntered = useRef(false); - const [shouldAnimate, setShouldAnimate] = useState(Boolean(!defaultOpen)); + const [shouldAnimate, setShouldAnimate] = useState(Boolean(!originalActive)); const hoverDelayTimeout = useRef(null); const hoverOutTimeout = useRef(null); const handleFocus = useCallback(() => { - if (originalActive === false) return; - - setActiveTrue(); + if (originalActive !== false) { + setActiveTrue(); + } }, [originalActive, setActiveTrue]); useEffect(() => { @@ -189,7 +179,7 @@ export function Tooltip({ id={id} preferredPosition={preferredPosition} activator={activatorNode} - active={open ?? active} + active={active} accessibilityLabel={accessibilityLabel} onClose={noop} preventInteraction={dismissOnMouseOut} diff --git a/polaris-react/src/components/Tooltip/tests/Tooltip.test.tsx b/polaris-react/src/components/Tooltip/tests/Tooltip.test.tsx index b7552e8ba0b..b17a4202214 100644 --- a/polaris-react/src/components/Tooltip/tests/Tooltip.test.tsx +++ b/polaris-react/src/components/Tooltip/tests/Tooltip.test.tsx @@ -35,94 +35,16 @@ describe('', () => { expect(tooltipActive.find(TooltipOverlay)).toContainReactComponent('div'); }); - it('renders initially when defaultOpen is true', () => { - const tooltip = mountWithApp( - - link content - , - ); - - expect(tooltip.find(TooltipOverlay)).toContainReactComponent('div'); - }); - it('does not render when active is false', () => { - const tooltip = mountWithApp( + const tooltipActive = mountWithApp( link content , ); - expect(tooltip.find(TooltipOverlay)).not.toContainReactComponent('div'); - }); - - it('does not render initially when defaultOpen is false', () => { - const tooltip = mountWithApp( - - link content - , - ); - - expect(tooltip.find(TooltipOverlay)).not.toContainReactComponent('div'); - }); - - it('renders when open is true', () => { - const tooltip = mountWithApp( - - link content - , - ); - - expect(tooltip.find(TooltipOverlay)).toContainReactComponent('div'); - }); - - it('does not render when open is false', () => { - const tooltip = mountWithApp( - - link content - , - ); - - expect(tooltip.find(TooltipOverlay)).not.toContainReactComponent('div'); - }); - - it('renders when open is true and active is false', () => { - const tooltip = mountWithApp( - - link content - , - ); - - expect(tooltip.find(TooltipOverlay)).toContainReactComponent('div'); - }); - - it('renders when open is true and defaultOpen is false', () => { - const tooltip = mountWithApp( - - link content - , - ); - - expect(tooltip.find(TooltipOverlay)).toContainReactComponent('div'); - }); - - it('does not render when open is false and active is true', () => { - const tooltip = mountWithApp( - - link content - , + expect(tooltipActive.find(TooltipOverlay)).not.toContainReactComponent( + 'div', ); - - expect(tooltip.find(TooltipOverlay)).not.toContainReactComponent('div'); - }); - - it('does not render when open is false and defaultOpen is true', () => { - const tooltip = mountWithApp( - - link content - , - ); - - expect(tooltip.find(TooltipOverlay)).not.toContainReactComponent('div'); }); it('does not render when active prop is updated to false', () => { @@ -139,23 +61,9 @@ describe('', () => { expect(tooltip.find(TooltipOverlay)).not.toContainReactComponent('div'); }); - it('renders when defaultOpen prop is updated to false', () => { - const tooltip = mountWithApp( - - link content - , - ); - - findWrapperComponent(tooltip)!.trigger('onMouseOver'); - expect(tooltip.find(TooltipOverlay)).toContainReactComponent('div'); - - tooltip.setProps({defaultOpen: false}); - expect(tooltip.find(TooltipOverlay)).toContainReactComponent('div'); - }); - it('passes preventInteraction to TooltipOverlay when dismissOnMouseOut is true', () => { const tooltip = mountWithApp( - + link content , ); @@ -210,7 +118,7 @@ describe('', () => { it('closes itself when escape is pressed on keyup', () => { const tooltip = mountWithApp( - +
Order #1001
, ); @@ -224,11 +132,11 @@ describe('', () => { }); }); - it('does not call onOpen initially when defaultOpen is true', () => { + it('does not call onOpen when initially activated', () => { const openSpy = jest.fn(); const tooltip = mountWithApp( @@ -243,11 +151,11 @@ describe('', () => { expect(openSpy).not.toHaveBeenCalled(); }); - it('calls onClose initially when defaultOpen is true and then closed', () => { + it('calls onClose when initially activated and then closed', () => { const closeSpy = jest.fn(); const tooltip = mountWithApp( @@ -308,7 +216,7 @@ describe('', () => { const closeSpy = jest.fn(); const tooltip = mountWithApp( - + link content , ); @@ -326,7 +234,7 @@ describe('', () => { const closeSpy = jest.fn(); const tooltip = mountWithApp( - + link content , ); @@ -347,7 +255,7 @@ describe('', () => { link content , @@ -359,7 +267,7 @@ describe('', () => { it("passes 'zIndexOverride' to TooltipOverlay", () => { const tooltip = mountWithApp( - + link content , ); diff --git a/polaris.shopify.com/pages/examples/tooltip-default.tsx b/polaris.shopify.com/pages/examples/tooltip-default.tsx index 1d1208f139b..ab114375d29 100644 --- a/polaris.shopify.com/pages/examples/tooltip-default.tsx +++ b/polaris.shopify.com/pages/examples/tooltip-default.tsx @@ -5,7 +5,7 @@ import {withPolarisExample} from '../../src/components/PolarisExampleWrapper'; function TooltipExample() { return (
- + Order #1001 diff --git a/polaris.shopify.com/pages/examples/tooltip-with-persistence-on-click.tsx b/polaris.shopify.com/pages/examples/tooltip-with-persistence-on-click.tsx index 118cb65201e..86063c2ef75 100644 --- a/polaris.shopify.com/pages/examples/tooltip-with-persistence-on-click.tsx +++ b/polaris.shopify.com/pages/examples/tooltip-with-persistence-on-click.tsx @@ -5,11 +5,7 @@ import {withPolarisExample} from '../../src/components/PolarisExampleWrapper'; function TooltipExample() { return (
- + Order #1001 diff --git a/polaris.shopify.com/pages/examples/tooltip-with-underline.tsx b/polaris.shopify.com/pages/examples/tooltip-with-underline.tsx index 31c347456e6..271f4f5f2c7 100644 --- a/polaris.shopify.com/pages/examples/tooltip-with-underline.tsx +++ b/polaris.shopify.com/pages/examples/tooltip-with-underline.tsx @@ -6,11 +6,7 @@ function TooltipExample() { return (
- + Order #1001