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

fix(select): fix multi select popovers within parent popover #733

Merged
merged 2 commits into from
Dec 13, 2024
Merged

fix(select): fix multi select popovers within parent popover #733

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a733001
fix(select): fix multi select popovers within parent popover
chrisadubois Dec 12, 2024
79c66f1
fix(types): add comments for types
chrisadubois Dec 12, 2024
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
81 changes: 81 additions & 0 deletions src/components/Popover/Popover.stories.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import SearchInput from '../SearchInput';
import List from '../List';
import AriaToolbarItem from '../AriaToolbarItem';
import ListItemBase from '../ListItemBase';
import Select from '../Select';

export default {
title: 'Momentum UI/Popover',
Expand Down Expand Up @@ -305,6 +306,84 @@ NestedPopover.args = {
),
};

const MultipleSelectPopoversInsidePopoverInteractiveContentManaged = Template<PopoverProps>(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

say that 3 times fast

Popover
).bind({});

MultipleSelectPopoversInsidePopoverInteractiveContentManaged.argTypes = { ...argTypes };

MultipleSelectPopoversInsidePopoverInteractiveContentManaged.args = {
trigger: 'click',
placement: PLACEMENTS.BOTTOM,
showArrow: true,
interactive: true,
variant: 'small',
color: COLORS.TERTIARY,
delay: [0, 0],
triggerComponent: (
<ButtonPill style={{ margin: '10rem auto', display: 'flex' }}>Click me!</ButtonPill>
),
children: (
<>
<Select popoverSingleOpenGroupId="select1" aria-label="select1">
<Item>
<Text tagName="p">option1</Text>
</Item>
<Item>
<Text tagName="p">option2</Text>
</Item>
</Select>
<Select popoverSingleOpenGroupId="select1" aria-label="select2">
<Item>
<Text tagName="p">option3</Text>
</Item>
<Item>
<Text tagName="p">option4</Text>
</Item>
</Select>
</>
),
};

const MultipleSelectPopoversInsidePopoverInteractiveContentUnmanaged = Template<PopoverProps>(
Popover
).bind({});

MultipleSelectPopoversInsidePopoverInteractiveContentUnmanaged.argTypes = { ...argTypes };

MultipleSelectPopoversInsidePopoverInteractiveContentUnmanaged.args = {
trigger: 'click',
placement: PLACEMENTS.BOTTOM,
showArrow: true,
interactive: true,
variant: 'small',
color: COLORS.TERTIARY,
delay: [0, 0],
triggerComponent: (
<ButtonPill style={{ margin: '10rem auto', display: 'flex' }}>Click me!</ButtonPill>
),
children: (
<>
<Select aria-label="select1">
<Item>
<Text tagName="p">option1</Text>
</Item>
<Item>
<Text tagName="p">option2</Text>
</Item>
</Select>
<Select aria-label="select2">
<Item>
<Text tagName="p">option3</Text>
</Item>
<Item>
<Text tagName="p">option4</Text>
</Item>
</Select>
</>
),
};

const WithMeetingListItem = Template<PopoverProps>(Popover).bind({});

WithMeetingListItem.argTypes = { ...argTypes };
Expand Down Expand Up @@ -591,6 +670,8 @@ export {
WithCloseButton,
Offset,
MultiplePopovers,
MultipleSelectPopoversInsidePopoverInteractiveContentManaged,
MultipleSelectPopoversInsidePopoverInteractiveContentUnmanaged,
NestedPopover,
AvatarExample,
Common,
Expand Down
3 changes: 2 additions & 1 deletion src/components/Popover/Popover.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ const Popover = forwardRef((props: Props, ref: ForwardedRef<HTMLElement>) => {
boundary = DEFAULTS.BOUNDARY,
hideOnEsc = DEFAULTS.HIDE_ON_ESC,
hideOnBlur = DEFAULTS.HIDE_ON_BLUR,
singleOpenGroupId = undefined,
isChildPopoverOpen = DEFAULTS.IS_CHILD_POPOVER_OPEN,
addBackdrop = DEFAULTS.ADD_BACKDROP,
focusBackOnTrigger: focusBackOnTriggerFromProps,
Expand Down Expand Up @@ -239,7 +240,7 @@ const Popover = forwardRef((props: Props, ref: ForwardedRef<HTMLElement>) => {
}}
animation={false}
delay={delay}
plugins={addTippyPlugins(hideOnEsc, hideOnBlur, addBackdrop)}
plugins={addTippyPlugins(hideOnEsc, hideOnBlur, addBackdrop, singleOpenGroupId)}
// add arrow height to default offset if arrow is shown:
offset={[offsetSkidding, showArrow ? ARROW_HEIGHT + offsetDistance : offsetDistance]}
{...{
Expand Down
5 changes: 5 additions & 0 deletions src/components/Popover/Popover.types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,11 @@ export type PopoverCommonStyleProps = {
*/
hideOnBlur?: boolean;

/**
* Used when provided by a select which renders a popover. This is used to ensure only one popover is open at a time.
*/
singleOpenGroupId?: string;

/**
* Manual control of if the Popover contains child elements that may be open, and not nested within the
* Popover. it's possible the focus may shift to something that in the DOM is not actually within the Popover,
Expand Down
35 changes: 35 additions & 0 deletions src/components/Popover/Popover.unit.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -828,6 +828,41 @@ describe('<Popover />', () => {
expect(modalContainer).toBeInTheDocument();
});

describe('singleOpenGroupId functionality', () => {
it('should only allow one popover to be open within the same group', async () => {
const user = userEvent.setup();

render(
<>
<Popover
triggerComponent={<ButtonSimple>Popover 1</ButtonSimple>}
singleOpenGroupId="group1"
>
<p>Content 1</p>
</Popover>
<Popover
triggerComponent={<ButtonSimple>Popover 2</ButtonSimple>}
singleOpenGroupId="group1"
>
<p>Content 2</p>
</Popover>
</>
);

// Open first popover
await openPopoverByClickingOnTriggerAndCheckContent(user, /Popover 1/i, /Content 1/i);

// Ensure first popover is open
expect(screen.queryByText('Content 2')).not.toBeInTheDocument();

// Open second popover
await openPopoverByClickingOnTriggerAndCheckContent(user, /Popover 2/i, /Content 2/i);

// Ensure first popover is closed and second is open
expect(screen.queryByText('Content 1')).not.toBeInTheDocument();
});
});

const nullRef = null;
const callbackRef = jest.fn();
const mutableRef = { current: null };
Expand Down
97 changes: 91 additions & 6 deletions src/components/Popover/Popover.utils.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,46 +2,131 @@ import { addTippyPlugins } from './Popover.utils';
import { hideOnEscPlugin } from './tippy-plugins/hideOnEscPlugin';
import { addBackdropPlugin } from './tippy-plugins/backdropPlugin';
import { hideOnBlurPlugin } from './tippy-plugins/hideOnBlurPlugin';
import { getSingleOpenPlugin } from './tippy-plugins/singleOpenPlugin';

describe('addTippyPlugins', () => {
it.each([
{ hideOnEsc: false, hideOnBlur: false, addBackdrop: false, expected: [] },
{ hideOnEsc: true, hideOnBlur: false, addBackdrop: false, expected: [hideOnEscPlugin] },
{
hideOnEsc: false,
hideOnBlur: false,
addBackdrop: false,
singleOpenGroupId: undefined,
expected: [],
},
{
hideOnEsc: true,
hideOnBlur: false,
addBackdrop: false,
singleOpenGroupId: undefined,
expected: [hideOnEscPlugin],
},
{
hideOnEsc: false,
hideOnBlur: true,
addBackdrop: false,
singleOpenGroupId: undefined,
expected: [hideOnBlurPlugin],
},
{ hideOnEsc: false, hideOnBlur: false, addBackdrop: true, expected: [addBackdropPlugin] },
{
hideOnEsc: false,
hideOnBlur: false,
addBackdrop: true,
singleOpenGroupId: undefined,
expected: [addBackdropPlugin],
},
{
hideOnEsc: true,
hideOnBlur: true,
addBackdrop: false,
singleOpenGroupId: undefined,
expected: [hideOnEscPlugin, hideOnBlurPlugin],
},
{
hideOnEsc: true,
hideOnBlur: false,
addBackdrop: true,
singleOpenGroupId: undefined,
expected: [hideOnEscPlugin, addBackdropPlugin],
},
{
hideOnEsc: false,
hideOnBlur: true,
addBackdrop: true,
singleOpenGroupId: undefined,
expected: [addBackdropPlugin, hideOnBlurPlugin],
},
{
hideOnEsc: true,
hideOnBlur: true,
addBackdrop: true,
singleOpenGroupId: undefined,
expected: [hideOnEscPlugin, addBackdropPlugin, hideOnBlurPlugin],
},
{
hideOnEsc: false,
hideOnBlur: false,
addBackdrop: false,
singleOpenGroupId: 'group1',
expected: [getSingleOpenPlugin('group1')],
},
{
hideOnEsc: true,
hideOnBlur: false,
addBackdrop: false,
singleOpenGroupId: 'group1',
expected: [hideOnEscPlugin, getSingleOpenPlugin('group1')],
},
{
hideOnEsc: false,
hideOnBlur: true,
addBackdrop: false,
singleOpenGroupId: 'group1',
expected: [hideOnBlurPlugin, getSingleOpenPlugin('group1')],
},
{
hideOnEsc: false,
hideOnBlur: false,
addBackdrop: true,
singleOpenGroupId: 'group1',
expected: [addBackdropPlugin, getSingleOpenPlugin('group1')],
},
{
hideOnEsc: true,
hideOnBlur: true,
addBackdrop: false,
singleOpenGroupId: 'group1',
expected: [hideOnEscPlugin, hideOnBlurPlugin, getSingleOpenPlugin('group1')],
},
{
hideOnEsc: true,
hideOnBlur: false,
addBackdrop: true,
singleOpenGroupId: 'group1',
expected: [hideOnEscPlugin, addBackdropPlugin, getSingleOpenPlugin('group1')],
},
{
hideOnEsc: false,
hideOnBlur: true,
addBackdrop: true,
singleOpenGroupId: 'group1',
expected: [addBackdropPlugin, hideOnBlurPlugin, getSingleOpenPlugin('group1')],
},
{
hideOnEsc: true,
hideOnBlur: true,
addBackdrop: true,
singleOpenGroupId: 'group1',
expected: [
hideOnEscPlugin,
addBackdropPlugin,
hideOnBlurPlugin,
getSingleOpenPlugin('group1'),
],
},
])(
'should return $expected when hideOnEsc is $hideOnEsc, hideOnBlur is $hideOnBlur, and addBackdrop is $addBackdrop',
({ hideOnEsc, hideOnBlur, addBackdrop, expected }) => {
const result = addTippyPlugins(hideOnEsc, hideOnBlur, addBackdrop);
'should return $expected when hideOnEsc is $hideOnEsc, hideOnBlur is $hideOnBlur, addBackdrop is $addBackdrop, and singleOpenGroupId is $singleOpenGroupId',
({ hideOnEsc, hideOnBlur, addBackdrop, singleOpenGroupId, expected }) => {
const result = addTippyPlugins(hideOnEsc, hideOnBlur, addBackdrop, singleOpenGroupId);
expect(result).toEqual(expected);
}
);
Expand Down
7 changes: 6 additions & 1 deletion src/components/Popover/Popover.utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,13 @@ import type { Plugin } from 'tippy.js';
import { hideOnEscPlugin } from './tippy-plugins/hideOnEscPlugin';
import { addBackdropPlugin } from './tippy-plugins/backdropPlugin';
import { hideOnBlurPlugin } from './tippy-plugins/hideOnBlurPlugin';
import { getSingleOpenPlugin } from './tippy-plugins/singleOpenPlugin';

export const addTippyPlugins = (
hideOnEsc: boolean,
hideOnBlur: boolean,
addBackdrop: boolean
addBackdrop: boolean,
singleOpenGroupId?: string
): Array<Plugin> => {
const plugins = [];
if (hideOnEsc) {
Expand All @@ -18,5 +20,8 @@ export const addTippyPlugins = (
if (hideOnBlur) {
plugins.push(hideOnBlurPlugin);
}
if (singleOpenGroupId) {
plugins.push(getSingleOpenPlugin(singleOpenGroupId));
}
return plugins;
};
Loading
Loading