Skip to content

Commit

Permalink
Fix/dropdown click outside (#1615)
Browse files Browse the repository at this point in the history
  • Loading branch information
@moathabuhamad-cengage
moathabuhamad-cengage authored Jan 27, 2025
1 parent 29065e3 commit 4c9fa50
Show file tree
Hide file tree
Showing 9 changed files with 381 additions and 67 deletions.
5 changes: 5 additions & 0 deletions .changeset/fix-Dropdown-click-outside.md
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'react-magma-dom': patch
---

fix(Dropdown): fix click outside behavior in Dropdown
47 changes: 47 additions & 0 deletions packages/react-magma-dom/src/components/Dropdown/Dropdown.stories.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -623,3 +623,50 @@ export const FlippedItems = args => {
</>
);
};

const CustomRefTemplate: Story<DropdownProps> = args => {
const buttonRef = React.useRef<HTMLButtonElement>(null);
const splitButtonRef = React.useRef<HTMLButtonElement>(null);

function handleClose(event: React.SyntheticEvent) {
buttonRef.current?.focus();
}

function handleSplitClose(event: React.SyntheticEvent) {
splitButtonRef.current?.focus();
}

return (
<div
style={{
margin: '150px auto',
textAlign: 'center',
gap: '20px',
display: 'flex',
justifyContent: 'center',
}}
>
<Dropdown {...args} onClose={handleClose}>
<DropdownButton ref={buttonRef}>Basic Dropdown</DropdownButton>
<DropdownContent>
<DropdownMenuItem>Menu item 1</DropdownMenuItem>
<DropdownMenuItem>Menu item number two</DropdownMenuItem>
</DropdownContent>
</Dropdown>
<Dropdown {...args} onClose={handleSplitClose}>
<DropdownSplitButton aria-label="Split" ref={splitButtonRef}>
Split Dropdown
</DropdownSplitButton>
<DropdownContent>
<DropdownMenuItem>Menu item 1</DropdownMenuItem>
<DropdownMenuItem>Menu item number two</DropdownMenuItem>
</DropdownContent>
</Dropdown>
</div>
);
};

export const CustomRef = CustomRefTemplate.bind({});
CustomRef.args = {
...Default.args,
};
157 changes: 144 additions & 13 deletions packages/react-magma-dom/src/components/Dropdown/Dropdown.test.js
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import React from 'react';
import { AsteriskIcon , RestaurantMenuIcon } from 'react-magma-icons';
import { AsteriskIcon, RestaurantMenuIcon } from 'react-magma-icons';
import { Dropdown } from '.';
import {
DropdownContent,
Expand All @@ -20,7 +20,7 @@ import { Modal } from '../Modal';
import { magma } from '../../theme/magma';
import { transparentize } from 'polished';

import { act, render, fireEvent, getByTestId } from '@testing-library/react';
import { act, render, fireEvent, getByTestId, getByLabelText } from '@testing-library/react';
import userEvent from '@testing-library/user-event';

describe('Dropdown', () => {
Expand All @@ -30,8 +30,8 @@ describe('Dropdown', () => {
<Dropdown testId={testId}>
<DropdownButton>Toggle me</DropdownButton>
<DropdownContent>
<DropdownMenuItem onClick={() => {}}>Menu item 1</DropdownMenuItem>
<DropdownMenuItem onClick={() => {}}>
<DropdownMenuItem onClick={() => { }}>Menu item 1</DropdownMenuItem>
<DropdownMenuItem onClick={() => { }}>
Menu item number two
</DropdownMenuItem>
</DropdownContent>
Expand All @@ -57,10 +57,10 @@ describe('Dropdown', () => {
<Dropdown>
<DropdownButton>Toggle me</DropdownButton>
<DropdownContent>
<DropdownMenuItem onClick={() => {}}>Menu item 1</DropdownMenuItem>
<OptionalDropdownItem onClick={() => {}} toggle />
<DropdownMenuItem onClick={() => { }}>Menu item 1</DropdownMenuItem>
<OptionalDropdownItem onClick={() => { }} toggle />
<div>
<DropdownMenuItem onClick={() => {}}>FAQ</DropdownMenuItem>
<DropdownMenuItem onClick={() => { }}>FAQ</DropdownMenuItem>
</div>
</DropdownContent>
</Dropdown>
Expand Down Expand Up @@ -392,6 +392,104 @@ describe('Dropdown', () => {
jest.useRealTimers();
});

it('should open one dropdown at a time, close the previous one, and close when clicking outside', () => {
jest.useFakeTimers();

const onClose1 = jest.fn();
const onClose2 = jest.fn();
const { getByText, getByTestId } = render(
<>
<Dropdown testId="dropdown1" onClose={onClose1}>
<DropdownButton>Toggle me 1</DropdownButton>
<DropdownContent testId="dropdown1Content">
<DropdownMenuItem>Menu item 1</DropdownMenuItem>
</DropdownContent>
</Dropdown>
<Dropdown testId="dropdown2" onClose={onClose2}>
<DropdownButton>Toggle me 2</DropdownButton>
<DropdownContent testId="dropdown2Content">
<DropdownMenuItem>Menu item 2</DropdownMenuItem>
</DropdownContent>
</Dropdown>
</>
);

expect(getByTestId('dropdown1Content')).toHaveStyleRule('display', 'none');
expect(getByTestId('dropdown2Content')).toHaveStyleRule('display', 'none');

// Open the first dropdown
userEvent.click(getByText('Toggle me 1'));
expect(getByTestId('dropdown1Content')).toHaveStyleRule('display', 'block');
expect(getByTestId('dropdown2Content')).toHaveStyleRule('display', 'none');

// Open the second dropdown, which should close the first one
userEvent.click(getByText('Toggle me 2'));

expect(onClose1).toHaveBeenCalled();
expect(getByTestId('dropdown1Content')).toHaveStyleRule('display', 'none');
expect(getByTestId('dropdown2Content')).toHaveStyleRule('display', 'block');

// Close the second dropdown by clicking outside
fireEvent.mouseDown(document.body);
act(jest.runAllTimers);

expect(onClose2).toHaveBeenCalled();
expect(getByTestId('dropdown2Content')).toHaveStyleRule('display', 'none');

jest.useRealTimers();
});

it('should open one splitdropdown at a time, close the previous one, and close when clicking outside', () => {
jest.useFakeTimers();

const onClose1 = jest.fn();
const onClose2 = jest.fn();
const { getAllByLabelText, getByTestId } = render(
<>
<Dropdown testId="dropdown1" onClose={onClose1}>
<DropdownSplitButton
aria-label="Split"
>Toggle me 1</DropdownSplitButton>
<DropdownContent testId="dropdown1Content">
<DropdownMenuItem>Menu item 1</DropdownMenuItem>
</DropdownContent>
</Dropdown>
<Dropdown testId="dropdown2" onClose={onClose2}>
<DropdownSplitButton
aria-label="Split"
>Toggle me 2</DropdownSplitButton>
<DropdownContent testId="dropdown2Content">
<DropdownMenuItem>Menu item 2</DropdownMenuItem>
</DropdownContent>
</Dropdown>
</>
);

expect(getByTestId('dropdown1Content')).toHaveStyleRule('display', 'none');
expect(getByTestId('dropdown2Content')).toHaveStyleRule('display', 'none');

// Open the first dropdown
userEvent.click(getAllByLabelText('Split')[0]);
expect(getByTestId('dropdown1Content')).toHaveStyleRule('display', 'block');
expect(getByTestId('dropdown2Content')).toHaveStyleRule('display', 'none');

// Open the second dropdown, which should close the first one
userEvent.click(getAllByLabelText('Split')[1]);

expect(onClose1).toHaveBeenCalled();
expect(getByTestId('dropdown1Content')).toHaveStyleRule('display', 'none');
expect(getByTestId('dropdown2Content')).toHaveStyleRule('display', 'block');

// Close the second dropdown by clicking outside
fireEvent.mouseDown(document.body);
act(jest.runAllTimers);

expect(onClose2).toHaveBeenCalled();
expect(getByTestId('dropdown2Content')).toHaveStyleRule('display', 'none');

jest.useRealTimers();
});

it('should close the menu when escape key is pressed', () => {
const { getByText, getByTestId } = render(
<Dropdown testId="dropdown">
Expand Down Expand Up @@ -425,9 +523,9 @@ describe('Dropdown', () => {
<Dropdown testId="dropdown">
<DropdownButton>Toggle me</DropdownButton>
<DropdownContent>
<DropdownMenuItem onClick={() => {}}>Menu item 1</DropdownMenuItem>
<DropdownMenuItem onClick={() => { }}>Menu item 1</DropdownMenuItem>
<DropdownDivider />
<DropdownMenuItem onClick={() => {}}>Menu item 2</DropdownMenuItem>
<DropdownMenuItem onClick={() => { }}>Menu item 2</DropdownMenuItem>
</DropdownContent>
</Dropdown>
);
Expand Down Expand Up @@ -456,9 +554,9 @@ describe('Dropdown', () => {
<Dropdown testId="dropdown">
<DropdownButton>Toggle me</DropdownButton>
<DropdownContent>
<DropdownMenuItem onClick={() => {}}>Menu item 1</DropdownMenuItem>
<DropdownMenuItem onClick={() => { }}>Menu item 1</DropdownMenuItem>
<DropdownDivider />
<DropdownMenuItem onClick={() => {}}>Menu item 2</DropdownMenuItem>
<DropdownMenuItem onClick={() => { }}>Menu item 2</DropdownMenuItem>
</DropdownContent>
</Dropdown>
);
Expand Down Expand Up @@ -642,13 +740,46 @@ describe('Dropdown', () => {
expect(onClick).toHaveBeenCalled();
});

it('should call onClose for the previously opened dropdown when a new one is opened', async () => {
const onClose1 = jest.fn();
const onClose2 = jest.fn();

const { getByText } = render(
<>
<Dropdown testId="dropdown1" onClose={onClose1}>
<DropdownButton>Toggle Dropdown 1</DropdownButton>
<DropdownContent>
<DropdownMenuItem>Item 1</DropdownMenuItem>
</DropdownContent>
</Dropdown>
<Dropdown testId="dropdown2" onClose={onClose2}>
<DropdownButton>Toggle Dropdown 2</DropdownButton>
<DropdownContent>
<DropdownMenuItem>Item 2</DropdownMenuItem>
</DropdownContent>
</Dropdown>
</>
);

// Open the first dropdown
userEvent.click(getByText('Toggle Dropdown 1'));
expect(onClose1).not.toHaveBeenCalled();

// Open the second dropdown
userEvent.click(getByText('Toggle Dropdown 2'));

// Verify that the first dropdown's onClose is called
expect(onClose1).toHaveBeenCalled();
expect(onClose2).not.toHaveBeenCalled(); // Second dropdown is still open
});

it('should render a dropdown with an active item', async () => {
const { container, getByText } = render(
<Dropdown activeIndex={1}>
<DropdownButton>Toggle</DropdownButton>
<DropdownContent>
<DropdownMenuItem onClick={() => {}}>aaa</DropdownMenuItem>
<DropdownMenuItem onClick={() => {}}>bbb</DropdownMenuItem>
<DropdownMenuItem onClick={() => { }}>aaa</DropdownMenuItem>
<DropdownMenuItem onClick={() => { }}>bbb</DropdownMenuItem>
</DropdownContent>
</Dropdown>
);
Expand Down
83 changes: 54 additions & 29 deletions packages/react-magma-dom/src/components/Dropdown/Dropdown.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ export const Dropdown = React.forwardRef<HTMLDivElement, DropdownProps>(
);

const ownRef = React.useRef<any>();
const toggleRef = React.useRef<HTMLButtonElement>();
const toggleRef = React.useRef<HTMLButtonElement>(null);
const menuRef = React.useRef<any>([]);
const dropdownButtonId = React.useRef<string>('');

Expand All @@ -160,7 +160,7 @@ export const Dropdown = React.forwardRef<HTMLDivElement, DropdownProps>(

if (filteredItems.length > 0) {
setTimeout(() => {
filteredItems[0].current && filteredItems[0].current.focus();
filteredItems[0]?.current?.focus();
}, 0);
} else {
setTimeout(() => {
Expand All @@ -171,10 +171,10 @@ export const Dropdown = React.forwardRef<HTMLDivElement, DropdownProps>(
onOpen && typeof onOpen === 'function' && onOpen();
}

function closeDropdown(event) {
function closeDropdown(event: React.SyntheticEvent<Element, Event>) {
setIsOpen(false);

toggleRef.current.focus();
toggleRef.current?.focus();

onClose && typeof onClose === 'function' && onClose(event);
}
Expand Down Expand Up @@ -283,32 +283,57 @@ export const Dropdown = React.forwardRef<HTMLDivElement, DropdownProps>(
changePlacement(dropDirection, alignment);
}, [dropDirection, alignment]);

const contextValue = React.useMemo(
() => ({
activeItemIndex,
alignment,
closeDropdown,
dropdownButtonId,
dropDirection,
floatingStyles,
handleDropdownBlur,
itemRefArray,
isFixedWidth: !!width,
isOpen,
isInverse,
maxHeight: maxHeightString,
menuRef,
openDropdown,
registerDropdownMenuItem,
setActiveItemIndex,
setIsOpen,
setReference: refs.setReference,
setFloating: refs.setFloating,
toggleRef,
width: widthString,
}),
[
activeItemIndex,
alignment,
closeDropdown,
dropdownButtonId,
dropDirection,
floatingStyles,
handleDropdownBlur,
itemRefArray,
width,
isOpen,
isInverse,
maxHeightString,
menuRef,
openDropdown,
registerDropdownMenuItem,
setActiveItemIndex,
setIsOpen,
refs.setReference,
refs.setFloating,
toggleRef,
widthString,
]
);

return (
<DropdownContext.Provider
value={{
activeItemIndex,
alignment,
closeDropdown,
dropdownButtonId,
dropDirection,
floatingStyles,
handleDropdownBlur,
itemRefArray,
isFixedWidth: !!width,
isOpen,
isInverse,
maxHeight: maxHeightString,
menuRef,
openDropdown,
registerDropdownMenuItem,
setActiveItemIndex,
setIsOpen,
setReference: refs.setReference,
setFloating: refs.setFloating,
toggleRef,
width: widthString,
}}
>
<DropdownContext.Provider value={contextValue}>
<Container
{...other}
ref={ref}
Expand Down
Loading

2 comments on commit 4c9fa50

@github-actions
Copy link
Contributor

Choose a reason for hiding this comment

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

🚀 Deployed on https://next--upbeat-sinoussi-f675aa.netlify.app

@github-actions
Copy link
Contributor

Choose a reason for hiding this comment

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

🚀 Deployed on https://storybook-preview-dev--upbeat-sinoussi-f675aa.netlify.app

Please sign in to comment.