-
Notifications
You must be signed in to change notification settings - Fork 13
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/dropdown expandable menu #1129
Changes from 2 commits
8583f96
d58121b
923b459
4a2ddd4
41a7ba7
6ab7930
8eed276
b6f4735
d8970d8
4fc3b4e
55898ea
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
--- | ||
'react-magma-dom': minor | ||
--- | ||
|
||
feat(DropdownExpandableMenu): A new menu item display for the Dropdown component which enables expandable lists by one level. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,19 +1,26 @@ | ||
import React from 'react'; | ||
import { AsteriskIcon } from 'react-magma-icons'; | ||
import { Dropdown } from '.'; | ||
import { DropdownContent } from './DropdownContent'; | ||
import { DropdownDivider } from './DropdownDivider'; | ||
import { DropdownHeader } from './DropdownHeader'; | ||
import { DropdownMenuItem } from './DropdownMenuItem'; | ||
import { DropdownMenuGroup } from './DropdownMenuGroup'; | ||
import { DropdownSplitButton } from './DropdownSplitButton'; | ||
import { DropdownButton } from './DropdownButton'; | ||
import { DropdownMenuNavItem } from './DropdownMenuNavItem'; | ||
import { | ||
DropdownContent, | ||
DropdownDivider, | ||
DropdownHeader, | ||
DropdownMenuItem, | ||
DropdownMenuGroup, | ||
DropdownSplitButton, | ||
DropdownButton, | ||
DropdownMenuNavItem, | ||
DropdownExpandableMenuGroup, | ||
DropdownExpandableMenuItem, | ||
DropdownExpandableMenuButton, | ||
DropdownExpandableMenuPanel, | ||
} from './'; | ||
import { Modal } from '../Modal'; | ||
import { magma } from '../../theme/magma'; | ||
import { RestaurantMenuIcon } from 'react-magma-icons'; | ||
import { transparentize } from 'polished'; | ||
|
||
import { act, render, fireEvent } from '@testing-library/react'; | ||
import { act, render, fireEvent, getByTestId } from '@testing-library/react'; | ||
import userEvent from '@testing-library/user-event'; | ||
|
||
describe('Dropdown', () => { | ||
|
@@ -827,4 +834,262 @@ describe('Dropdown', () => { | |
jest.useRealTimers(); | ||
}); | ||
}); | ||
|
||
describe('dropdown with expandable menu', () => { | ||
const testId = 'expandable group'; | ||
const testId2 = 'expandable item'; | ||
const testId3 = 'expandable button'; | ||
const testId4 = 'expandable panel'; | ||
|
||
it('should render an expandable menu group', () => { | ||
const { getByTestId } = render( | ||
<Dropdown> | ||
<DropdownButton>Expandable Items Dropdown</DropdownButton> | ||
<DropdownContent> | ||
<DropdownExpandableMenuGroup testId={testId}> | ||
<DropdownExpandableMenuItem testId={testId2}> | ||
<DropdownExpandableMenuButton testId={testId3}> | ||
Pasta | ||
</DropdownExpandableMenuButton> | ||
</DropdownExpandableMenuItem> | ||
</DropdownExpandableMenuGroup> | ||
</DropdownContent> | ||
</Dropdown> | ||
); | ||
expect(getByTestId(testId)).toBeInTheDocument(); | ||
expect(getByTestId(testId2)).toBeInTheDocument(); | ||
expect(getByTestId(testId3)).toBeInTheDocument(); | ||
}); | ||
|
||
it('should render an expandable menu group with icons', () => { | ||
const { getByText } = render( | ||
<Dropdown> | ||
<DropdownButton>Expandable Items Dropdown</DropdownButton> | ||
<DropdownContent> | ||
<DropdownExpandableMenuGroup> | ||
<DropdownExpandableMenuItem> | ||
<DropdownExpandableMenuButton icon={<RestaurantMenuIcon />}> | ||
Pasta | ||
</DropdownExpandableMenuButton> | ||
</DropdownExpandableMenuItem> | ||
</DropdownExpandableMenuGroup> | ||
</DropdownContent> | ||
</Dropdown> | ||
); | ||
expect(getByText('Pasta').querySelector('svg')).toBeInTheDocument(); | ||
}); | ||
|
||
it('should render an expanded panel of menu items when the DropdownExpandableMenuButton is clicked', () => { | ||
const { getByTestId, getByText } = render( | ||
<Dropdown> | ||
<DropdownButton>Expandable Items Dropdown</DropdownButton> | ||
<DropdownContent> | ||
<DropdownExpandableMenuGroup> | ||
<DropdownExpandableMenuItem> | ||
<DropdownExpandableMenuButton> | ||
Pasta | ||
</DropdownExpandableMenuButton> | ||
<DropdownExpandableMenuPanel testId={testId4}> | ||
<DropdownMenuItem>Fresh</DropdownMenuItem> | ||
<DropdownMenuItem>Processed</DropdownMenuItem> | ||
</DropdownExpandableMenuPanel> | ||
</DropdownExpandableMenuItem> | ||
</DropdownExpandableMenuGroup> | ||
</DropdownContent> | ||
</Dropdown> | ||
); | ||
|
||
fireEvent.click(getByText('Pasta')); | ||
|
||
expect(getByTestId(testId4)).toBeInTheDocument(); | ||
}); | ||
|
||
it('should close an expanded panel of menu items when the DropdownExpandableMenuButton is clicked', () => { | ||
const { getByTestId, getByText } = render( | ||
<Dropdown> | ||
<DropdownButton>Expandable Items Dropdown</DropdownButton> | ||
<DropdownContent> | ||
<DropdownExpandableMenuGroup> | ||
<DropdownExpandableMenuItem> | ||
<DropdownExpandableMenuButton> | ||
Pasta | ||
</DropdownExpandableMenuButton> | ||
<DropdownExpandableMenuPanel testId={testId4}> | ||
<DropdownMenuItem>Fresh</DropdownMenuItem> | ||
<DropdownMenuItem>Processed</DropdownMenuItem> | ||
</DropdownExpandableMenuPanel> | ||
</DropdownExpandableMenuItem> | ||
</DropdownExpandableMenuGroup> | ||
</DropdownContent> | ||
</Dropdown> | ||
); | ||
|
||
fireEvent.click(getByText('Pasta')); | ||
|
||
expect(getByTestId(testId4)).toBeInTheDocument(); | ||
|
||
fireEvent.click(getByText('Pasta')); | ||
|
||
expect(getByText('Fresh')).not.toBeVisible(); | ||
}); | ||
|
||
it('should have a default expanded item set by the user with defaultIndex', () => { | ||
const { getByTestId, getByText, queryByTestId } = render( | ||
<Dropdown> | ||
<DropdownButton>Expandable Items Dropdown</DropdownButton> | ||
<DropdownContent> | ||
<DropdownExpandableMenuGroup defaultIndex={[0]}> | ||
<DropdownExpandableMenuItem> | ||
<DropdownExpandableMenuButton> | ||
Pasta | ||
</DropdownExpandableMenuButton> | ||
<DropdownExpandableMenuPanel testId={testId4}> | ||
<DropdownMenuItem>Fresh</DropdownMenuItem> | ||
<DropdownMenuItem>Processed</DropdownMenuItem> | ||
</DropdownExpandableMenuPanel> | ||
</DropdownExpandableMenuItem> | ||
|
||
<DropdownExpandableMenuItem> | ||
<DropdownExpandableMenuButton> | ||
Bacon | ||
</DropdownExpandableMenuButton> | ||
<DropdownExpandableMenuPanel testId={`${testId4}-2`}> | ||
<DropdownMenuItem>Fresh</DropdownMenuItem> | ||
<DropdownMenuItem>Processed</DropdownMenuItem> | ||
</DropdownExpandableMenuPanel> | ||
</DropdownExpandableMenuItem> | ||
</DropdownExpandableMenuGroup> | ||
</DropdownContent> | ||
</Dropdown> | ||
); | ||
|
||
fireEvent.click(getByText('Expandable Items Dropdown')); | ||
|
||
expect(getByTestId(testId4)).toBeInTheDocument(); | ||
|
||
expect(queryByTestId(`${testId4}-2`)).not.toBeInTheDocument(); | ||
}); | ||
|
||
it('should have only allow one open menu item when isMulti is used', () => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's have separate tests for There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Anything not declaring the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would say yes for full coverage. |
||
const { getByTestId, getByText, queryByTestId } = render( | ||
<Dropdown> | ||
<DropdownButton>Expandable Items Dropdown</DropdownButton> | ||
<DropdownContent> | ||
<DropdownExpandableMenuGroup isMulti={false}> | ||
<DropdownExpandableMenuItem> | ||
<DropdownExpandableMenuButton> | ||
Pasta | ||
</DropdownExpandableMenuButton> | ||
<DropdownExpandableMenuPanel testId={testId4}> | ||
<DropdownMenuItem>Fresh</DropdownMenuItem> | ||
<DropdownMenuItem>Processed Stuff</DropdownMenuItem> | ||
</DropdownExpandableMenuPanel> | ||
</DropdownExpandableMenuItem> | ||
|
||
<DropdownExpandableMenuItem> | ||
<DropdownExpandableMenuButton> | ||
Bacon | ||
</DropdownExpandableMenuButton> | ||
<DropdownExpandableMenuPanel testId={`${testId4}-2`}> | ||
<DropdownMenuItem>Fresh</DropdownMenuItem> | ||
<DropdownMenuItem>Processed</DropdownMenuItem> | ||
</DropdownExpandableMenuPanel> | ||
</DropdownExpandableMenuItem> | ||
</DropdownExpandableMenuGroup> | ||
</DropdownContent> | ||
</Dropdown> | ||
); | ||
|
||
fireEvent.click(getByText('Expandable Items Dropdown')); | ||
|
||
fireEvent.click(getByText('Pasta')); | ||
|
||
expect(getByTestId(testId4)).toBeInTheDocument(); | ||
|
||
expect(queryByTestId(`${testId4}-2`)).not.toBeInTheDocument(); | ||
|
||
fireEvent.click(getByText('Bacon')); | ||
|
||
expect(getByTestId(`${testId4}-2`)).toBeInTheDocument(); | ||
|
||
expect(queryByTestId(testId4)).not.toBeVisible(); | ||
}); | ||
|
||
describe('dropdown with expandable menu styling', () => { | ||
it(`DropdownExpandableMenuPanel items should have standard padding if DropdownExpandableMenuButton doesn't have an icon`, () => { | ||
silvalaura marked this conversation as resolved.
Show resolved
Hide resolved
|
||
const { getByTestId, getByText } = render( | ||
<Dropdown> | ||
<DropdownButton>Expandable Items Dropdown</DropdownButton> | ||
<DropdownContent> | ||
<DropdownExpandableMenuGroup> | ||
<DropdownExpandableMenuItem> | ||
<DropdownExpandableMenuButton icon={<RestaurantMenuIcon />}> | ||
silvalaura marked this conversation as resolved.
Show resolved
Hide resolved
|
||
Pasta | ||
</DropdownExpandableMenuButton> | ||
<DropdownExpandableMenuPanel testId={testId4}> | ||
<DropdownMenuItem>Fresh</DropdownMenuItem> | ||
<DropdownMenuItem>Processed</DropdownMenuItem> | ||
</DropdownExpandableMenuPanel> | ||
</DropdownExpandableMenuItem> | ||
</DropdownExpandableMenuGroup> | ||
</DropdownContent> | ||
</Dropdown> | ||
); | ||
fireEvent.click(getByText('Pasta')); | ||
|
||
expect(getByText('Fresh')).toHaveStyleRule( | ||
'padding', | ||
`${magma.spaceScale.spacing03} ${magma.spaceScale.spacing05} ${magma.spaceScale.spacing03} 72px` | ||
); | ||
}); | ||
|
||
it(`DropdownExpandableMenuPanel items should have additional padding if DropdownExpandableMenuButton has an icon`, () => { | ||
const { getByTestId, getByText } = render( | ||
<Dropdown> | ||
<DropdownButton>Expandable Items Dropdown</DropdownButton> | ||
<DropdownContent> | ||
<DropdownExpandableMenuGroup> | ||
<DropdownExpandableMenuItem> | ||
<DropdownExpandableMenuButton> | ||
Pasta | ||
</DropdownExpandableMenuButton> | ||
<DropdownExpandableMenuPanel testId={testId4}> | ||
<DropdownMenuItem>Fresh</DropdownMenuItem> | ||
<DropdownMenuItem>Processed</DropdownMenuItem> | ||
</DropdownExpandableMenuPanel> | ||
</DropdownExpandableMenuItem> | ||
</DropdownExpandableMenuGroup> | ||
</DropdownContent> | ||
</Dropdown> | ||
); | ||
fireEvent.click(getByText('Pasta')); | ||
|
||
expect(getByText('Fresh')).toHaveStyleRule( | ||
'padding', | ||
`${magma.spaceScale.spacing03} ${magma.spaceScale.spacing05} ${magma.spaceScale.spacing03} ${magma.spaceScale.spacing08}` | ||
); | ||
}); | ||
it(`should support isInverse mode`, () => { | ||
const { getByTestId } = render( | ||
<Dropdown isInverse> | ||
<DropdownButton>Expandable Items Dropdown</DropdownButton> | ||
<DropdownContent> | ||
<DropdownExpandableMenuGroup | ||
testId={testId} | ||
></DropdownExpandableMenuGroup> | ||
</DropdownContent> | ||
</Dropdown> | ||
); | ||
|
||
expect(getByTestId(testId)).toHaveStyleRule( | ||
'background', | ||
'transparent' | ||
); | ||
expect(getByTestId(testId)).toHaveStyleRule( | ||
'color', | ||
magma.colors.neutral100 | ||
); | ||
}); | ||
}); | ||
}); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nitpick, but it would be helpful if these variables had useful names for readability:
You also use ``${testId4}-2
}
a few times, you could just add another variable for it