From 94e7f1f71c3772371c0d6c57336b531aeabd938f Mon Sep 17 00:00:00 2001 From: Gido Manders Date: Thu, 12 Dec 2024 12:31:36 +0100 Subject: [PATCH] fix: popover with openOnClick When openOnClick is used, the popover should close when clicked outside the popover. Changed Popover to use Tippy trigger property instead of a custom state with triggers to support click behaviour. --- src/core/Popover/Popover.stories.tsx | 82 +++++++++++++++---- src/core/Popover/Popover.test.tsx | 11 --- src/core/Popover/Popover.tsx | 36 +------- .../__snapshots__/Popover.test.tsx.snap | 66 ++++++++++++++- 4 files changed, 132 insertions(+), 63 deletions(-) diff --git a/src/core/Popover/Popover.stories.tsx b/src/core/Popover/Popover.stories.tsx index 8ea73051b..77bac40c0 100644 --- a/src/core/Popover/Popover.stories.tsx +++ b/src/core/Popover/Popover.stories.tsx @@ -17,9 +17,14 @@ storiesOf('core/Popover', module) .addDecorator((Story) => ( <> -

To be able to use Popover, you have to add @tippyjs/react to your dependencies:

+

+ To be able to use Popover, you have to add @tippyjs/react to your + dependencies: +

npm install --save @tippyjs/react -

You also have to add the stylesheet to your project

+

+ You also have to add the stylesheet to your project +

@import 'tippy.js/dist/tippy.css'; @@ -41,7 +46,13 @@ storiesOf('core/Popover', module) - Hover this icon!}> + + Hover this icon! + + } + > @@ -49,26 +60,47 @@ storiesOf('core/Popover', module) )) + .add('open on click', () => { + return ( + + + + + + ); + }) + .add('taking control', () => { - const [ isOpen, setIsOpen ] = useState(false); + const [isOpen, setIsOpen] = useState(false); return ( Status: {isOpen ? 'opened' : 'closed'} - +

- Note: you can take complete control over the Popover by using the isOpen prop. - Once you make it true or false the hover behavior will be disabled. + Note: you can take complete control over the Popover by using the{' '} + isOpen prop. Once you make it true or{' '} + false the hover behavior will be disabled.

); }) .add('on click outside', () => { - const [ isOpen, setIsOpen ] = useState(false); + const [isOpen, setIsOpen] = useState(false); return ( @@ -84,12 +116,13 @@ storiesOf('core/Popover', module)

- Note: you can take complete control over the Popover by using the isOpen prop. - Once you make it true or false the hover behavior will be disabled. + Note: you can take complete control over the Popover by using the{' '} + isOpen prop. Once you make it true or{' '} + false the hover behavior will be disabled.

- In combination with onClickOutside you can - close the popover when clicked anywhere outside the popover. + In combination with onClickOutside you can close the + popover when clicked anywhere outside the popover.

); @@ -133,15 +166,24 @@ storiesOf('core/Popover', module) Alignment-modifier - Hover me! } placement="right-start"> + Hover me! } + placement="right-start" + > right-start - Hover me! } placement="right"> + Hover me! } + placement="right" + > right - Hover me! } placement="right-end"> + Hover me! } + placement="right-end" + > right-end @@ -212,7 +254,11 @@ storiesOf('core/Popover', module) You can change that with the tag property - + Buttons are allowed too! @@ -221,9 +267,9 @@ storiesOf('core/Popover', module) )); function TinyCrud() { - const [ open, setOpen ] = useState(false); + const [open, setOpen] = useState(false); - const persons = [ 'aap', 'noot', 'mies' ]; + const persons = ['aap', 'noot', 'mies']; return ( diff --git a/src/core/Popover/Popover.test.tsx b/src/core/Popover/Popover.test.tsx index 139d603c3..bf9da8bc1 100644 --- a/src/core/Popover/Popover.test.tsx +++ b/src/core/Popover/Popover.test.tsx @@ -72,17 +72,6 @@ describe('Component: Popover', () => { }); describe('events', () => { - it('should call onClick when tag is clicked', () => { - const onClickSpy = jest.fn(); - render( - Click this} tag="button"> - Popover content - - ); - fireEvent.click(screen.getByText('Click this')); - expect(onClickSpy).toBeCalledTimes(1); - }); - it('should open on click when openOnClick is true', () => { render( Click this} tag="button"> diff --git a/src/core/Popover/Popover.tsx b/src/core/Popover/Popover.tsx index 987900f25..72115448c 100644 --- a/src/core/Popover/Popover.tsx +++ b/src/core/Popover/Popover.tsx @@ -1,4 +1,4 @@ -import React, { CSSProperties, useEffect, useState } from 'react'; +import React, { CSSProperties } from 'react'; import Tippy from '@tippyjs/react'; import { TippyPlacement } from '../types'; @@ -17,14 +17,6 @@ type Props = { */ openOnClick?: boolean; - /** - * Optional callback that gets triggered when the tag is clicked. - * When openOnClick is true, this component will take care of its visibility - * state, unless onClick is defined. - * This should be used when wanting to take complete control over the popover. - */ - onClick?: () => void; - /** * Optionally callback that gets triggered when clicked outside the popover. * Is useful for when wanting to take complete control over the popover. @@ -97,30 +89,15 @@ export function Popover({ className, isOpen, openOnClick, - onClick, onClickOutside, style, maxWidth }: Props) { const Tag = tag; - const [visible, setVisible] = useState(openOnClick ? !!isOpen : undefined); - - useEffect(() => { - setVisible(openOnClick ? !!isOpen : undefined); - }, [openOnClick, isOpen]); - - function tagClicked() { - if (onClick) { - onClick(); - } else if (openOnClick) { - setVisible(!visible); - } - } - return ( - + {target} diff --git a/src/core/Popover/__snapshots__/Popover.test.tsx.snap b/src/core/Popover/__snapshots__/Popover.test.tsx.snap index 403899953..8e28e62e6 100644 --- a/src/core/Popover/__snapshots__/Popover.test.tsx.snap +++ b/src/core/Popover/__snapshots__/Popover.test.tsx.snap @@ -17,7 +17,7 @@ exports[`Component: Popover ui default 1`] = ` exports[`Component: Popover ui open 1`] = `
+
+
`; exports[`Component: Popover ui with optional properties 1`] = `
+
+
`;