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.

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

feat(QTM-779): Add close functionality when clicks on elements other than the popover #587

Merged
merged 4 commits into from
Sep 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion components/Popover/Popover.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ const TriggerBlock = styled.div`
const Popover = ({
visible = false,
inverted = false,
closeOnClickOut = false,
onClose = () => {},
skin = 'neutral',
placement = 'top',
Expand All @@ -40,13 +41,21 @@ const Popover = ({
placement={placement}
onPopoverClose={() => handleVisible(false)}
inverted={inverted}
closeOnClickOut={closeOnClickOut}
skin={skin}
{...rest}
>
{children}
</Content>
)}
<TriggerBlock onClick={() => handleVisible(true)}>{trigger}</TriggerBlock>
<TriggerBlock
onClick={(event) => {
event.stopPropagation();
handleVisible(true);
}}
>
{trigger}
</TriggerBlock>
</Wrapper>
);
};
Expand All @@ -64,6 +73,7 @@ Popover.propTypes = {
skin: oneOf(['neutral', 'primary', 'success', 'warning', 'error']),
trigger: PropTypes.node.isRequired,
onClose: PropTypes.func,
closeOnClickOut: PropTypes.bool,
};

export default Popover;
28 changes: 28 additions & 0 deletions components/Popover/Popover.unit.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -78,4 +78,32 @@ describe('Popover component', () => {

expect(onCLoseEventMock).toHaveBeenCalled();
});

it('should call in the close event callback when the closeOnClickOut prop is true and there is a click outside the element', () => {
const onCLoseEventMock = jest.fn();
const sampleText = 'Another element on the page...';

render(
<div>
<Popover
skin="success"
trigger={POPOVER_TRIGGER_TEXT}
onClose={onCLoseEventMock}
closeOnClickOut
>
{POPOVER_TEXT}
</Popover>
<div>{sampleText}</div>
</div>,
);
const trigger = screen.getByText(POPOVER_TRIGGER_TEXT);
fireEvent.click(trigger);

expect(screen.queryByText(POPOVER_TEXT)).toBeInTheDocument();

const anyElement = screen.getByText(sampleText);
fireEvent.click(anyElement);

expect(screen.queryByText(POPOVER_TEXT)).not.toBeInTheDocument();
});
});
1 change: 1 addition & 0 deletions components/Popover/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { FC, ReactNode } from 'react';
export interface PopoverProps {
inverted?: boolean;
visible?: boolean;
closeOnClickOut?: boolean;
children: ReactNode[] | ReactNode;
placement?: 'top' | 'right' | 'bottom' | 'left';
skin?: 'neutral' | 'primary' | 'success' | 'warning' | 'error';
Expand Down
55 changes: 40 additions & 15 deletions components/Popover/sub-components/Content.jsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import styled, { css } from 'styled-components';
import PropTypes from 'prop-types';

import { useEffect, useRef } from 'react';
import { components, spacing, colors, breakpoints } from '../../shared/theme';
import getArrow from '../arrowProperties';
import Button from '../../Button';
Expand Down Expand Up @@ -131,24 +132,48 @@ const Content = ({
},
skin = 'neutral',
inverted = false,
closeOnClickOut = false,
...rest
}) => (
<PopoverContent
theme={theme}
inverted={inverted}
placement={placement}
skin={skin}
{...rest}
>
<PopoverChildren>{children}</PopoverChildren>
<CloseButton
skin={skin}
}) => {
const popoverRef = useRef(null);

// eslint-disable-next-line consistent-return
useEffect(() => {
if (closeOnClickOut) {
const onClickOutside = (event) => {
const element = event.target;

if (popoverRef.current && !popoverRef.current.contains(element)) {
onPopoverClose();
}
};

window.addEventListener('click', onClickOutside);
return () => {
window.removeEventListener('click', onClickOutside);
};
}
}, []);

return (
<PopoverContent
theme={theme}
inverted={inverted}
onClick={onPopoverClose}
/>
</PopoverContent>
);
placement={placement}
skin={skin}
ref={popoverRef}
{...rest}
>
<PopoverChildren>{children}</PopoverChildren>
<CloseButton
skin={skin}
theme={theme}
inverted={inverted}
onClick={onPopoverClose}
/>
</PopoverContent>
);
};

CloseButton.displayName = 'CloseButton';
PopoverChildren.displayName = 'PopoverChildren';
Expand Down
3 changes: 2 additions & 1 deletion stories/Popover/Popover.regression-test.story.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ const handleDirectionPosition = (direction) => {
};

const Template = (args) => {
const { direction, skin } = args;
const { direction, skin, closeOnClickOut } = args;

return (
<div>
Expand All @@ -19,6 +19,7 @@ const Template = (args) => {
skin={skin}
trigger={<Button>Popover trigger</Button>}
visible
closeOnClickOut={closeOnClickOut}
>
Some text
</Popover>
Expand Down