-
Notifications
You must be signed in to change notification settings - Fork 901
chore(site): refactor filter component to be more extendable #13688
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
Changes from 8 commits
d222f72
050f2c9
d4c183b
0706238
60aa016
08b0a82
c7e15ec
2e5880f
3fae8d4
06ae934
0d83bc8
59bd176
928209c
d344d9c
ebcbaa5
50a6ce4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,146 @@ | ||
import { action } from "@storybook/addon-actions"; | ||
import type { Meta, StoryObj } from "@storybook/react"; | ||
import { userEvent, within, expect } from "@storybook/test"; | ||
import { useState } from "react"; | ||
import { UserAvatar } from "components/UserAvatar/UserAvatar"; | ||
import { withDesktopViewport } from "testHelpers/storybook"; | ||
import { | ||
SelectFilter, | ||
SelectFilterSearch, | ||
type SelectFilterOption, | ||
} from "./SelectFilter"; | ||
|
||
const options: SelectFilterOption[] = Array.from({ length: 50 }, (_, i) => ({ | ||
startIcon: <UserAvatar username={`username ${i}`} size="xs" />, | ||
BrunoQuaresma marked this conversation as resolved.
Show resolved
Hide resolved
|
||
label: `Option ${i + 1}`, | ||
value: `option-${i + 1}`, | ||
})); | ||
|
||
const meta: Meta<typeof SelectFilter> = { | ||
title: "components/SelectFilter", | ||
component: SelectFilter, | ||
args: { | ||
options, | ||
placeholder: "All options", | ||
}, | ||
decorators: [withDesktopViewport], | ||
render: function SelectFilterWithState(args) { | ||
const [selectedOption, setSelectedOption] = useState< | ||
SelectFilterOption | undefined | ||
>(args.selectedOption); | ||
return ( | ||
<SelectFilter | ||
{...args} | ||
selectedOption={selectedOption} | ||
onSelect={setSelectedOption} | ||
/> | ||
); | ||
}, | ||
play: async ({ canvasElement }) => { | ||
const canvas = within(canvasElement); | ||
const button = canvas.getByRole("button"); | ||
await userEvent.click(button); | ||
}, | ||
}; | ||
|
||
export default meta; | ||
type Story = StoryObj<typeof SelectFilter>; | ||
|
||
export const Closed: Story = { | ||
play: undefined, | ||
}; | ||
|
||
export const Open: Story = {}; | ||
|
||
export const Selected: Story = { | ||
args: { | ||
selectedOption: options[25], | ||
}, | ||
}; | ||
|
||
export const WithSearch: Story = { | ||
args: { | ||
selectedOption: options[25], | ||
search: ( | ||
<SelectFilterSearch | ||
value="" | ||
onChange={action("onSearch")} | ||
placeholder="Search options..." | ||
/> | ||
), | ||
}, | ||
}; | ||
|
||
export const LoadingOptions: Story = { | ||
args: { | ||
options: undefined, | ||
}, | ||
}; | ||
|
||
export const NoOptionsFound: Story = { | ||
args: { | ||
options: [], | ||
}, | ||
}; | ||
|
||
export const SelectingOption: Story = { | ||
play: async ({ canvasElement }) => { | ||
const canvas = within(canvasElement); | ||
const button = canvas.getByRole("button"); | ||
await userEvent.click(button); | ||
const option = canvas.getByText("Option 25"); | ||
await userEvent.click(option); | ||
await expect(button).toHaveTextContent("Option 25"); | ||
}, | ||
}; | ||
|
||
export const UnselectingOption: Story = { | ||
args: { | ||
selectedOption: options[25], | ||
}, | ||
play: async ({ canvasElement }) => { | ||
const canvas = within(canvasElement); | ||
const button = canvas.getByRole("button"); | ||
await userEvent.click(button); | ||
const menu = canvasElement.querySelector<HTMLElement>("[role=menu]")!; | ||
const option = within(menu).getByText("Option 26"); | ||
await userEvent.click(option); | ||
await expect(button).toHaveTextContent("All options"); | ||
}, | ||
}; | ||
|
||
export const SearchingOption: Story = { | ||
render: function SelectFilterWithSearch(args) { | ||
const [selectedOption, setSelectedOption] = useState< | ||
SelectFilterOption | undefined | ||
>(args.selectedOption); | ||
const [search, setSearch] = useState(""); | ||
const visibleOptions = options.filter((option) => | ||
option.value.includes(search), | ||
); | ||
|
||
return ( | ||
<SelectFilter | ||
{...args} | ||
selectedOption={selectedOption} | ||
onSelect={setSelectedOption} | ||
options={visibleOptions} | ||
search={ | ||
<SelectFilterSearch | ||
value={search} | ||
onChange={setSearch} | ||
placeholder="Search options..." | ||
inputProps={{ "aria-label": "Search options" }} | ||
/> | ||
} | ||
/> | ||
); | ||
}, | ||
play: async ({ canvasElement }) => { | ||
const canvas = within(canvasElement); | ||
const button = canvas.getByRole("button"); | ||
await userEvent.click(button); | ||
const search = canvas.getByLabelText("Search options"); | ||
await userEvent.type(search, "option-2"); | ||
}, | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,117 @@ | ||
import visuallyHidden from "@mui/utils/visuallyHidden"; | ||
import { useState, type FC, type ReactNode } from "react"; | ||
import { Loader } from "components/Loader/Loader"; | ||
import { | ||
SelectMenu, | ||
SelectMenuTrigger, | ||
SelectMenuButton, | ||
SelectMenuContent, | ||
SelectMenuSearch, | ||
SelectMenuList, | ||
SelectMenuItem, | ||
SelectMenuIcon, | ||
} from "components/SelectMenu/SelectMenu"; | ||
|
||
const BASE_WIDTH = 200; | ||
const POPOVER_WIDTH = 320; | ||
|
||
export type SelectFilterOption = { | ||
startIcon?: ReactNode; | ||
label: string; | ||
value: string; | ||
}; | ||
|
||
export type SelectFilterProps = { | ||
options: SelectFilterOption[] | undefined; | ||
selectedOption?: SelectFilterOption; | ||
// Used to add a accessibility label to the select | ||
label: string; | ||
// Used when there is no option selected | ||
placeholder: string; | ||
// Used to customize the empty state message | ||
emptyText?: string; | ||
onSelect: (option: SelectFilterOption | undefined) => void; | ||
// SelectFilterSearch element | ||
search?: ReactNode; | ||
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. Could this prop be renamed to make it more clear that it's meant to be used with 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. When using |
||
}; | ||
|
||
export const SelectFilter: FC<SelectFilterProps> = ({ | ||
label, | ||
options, | ||
selectedOption, | ||
onSelect, | ||
placeholder, | ||
emptyText, | ||
search, | ||
}) => { | ||
const [open, setOpen] = useState(false); | ||
|
||
return ( | ||
<SelectMenu open={open} onOpenChange={setOpen}> | ||
<SelectMenuTrigger> | ||
<SelectMenuButton | ||
startIcon={selectedOption?.startIcon} | ||
css={{ width: BASE_WIDTH }} | ||
> | ||
{selectedOption?.label ?? placeholder} | ||
<span css={{ ...visuallyHidden }}>{label}</span> | ||
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. Can you explain what you were going for with this? From how I'm reading it, if the If it's okay for the main visual label text to be hidden from screen readers, this could be rewritten as <span aria-hidden>{selectedOption?.label ?? placeholder}</span> This hides it from screen readers, but keeps the content on the page. Normally this is a really bad idea, but as long as we have the backup label guaranteed to be defined, I think it's okay Edit: 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 didn't notice it 🤦 My idea is, since this is a select component, I think having a label like "Select a user" is better than having the selected value as the label "User 2" for example. 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 used |
||
</SelectMenuButton> | ||
</SelectMenuTrigger> | ||
<SelectMenuContent | ||
horizontal="right" | ||
css={{ | ||
"& .MuiPaper-root": { | ||
// When including search, we aim for the width to be as wide as | ||
// possible. | ||
width: search ? "100%" : undefined, | ||
maxWidth: POPOVER_WIDTH, | ||
minWidth: BASE_WIDTH, | ||
}, | ||
}} | ||
> | ||
{search} | ||
{options ? ( | ||
options.length > 0 ? ( | ||
<SelectMenuList> | ||
{options.map((o) => { | ||
const isSelected = o.value === selectedOption?.value; | ||
return ( | ||
<SelectMenuItem | ||
key={o.value} | ||
selected={isSelected} | ||
onClick={() => { | ||
setOpen(false); | ||
onSelect(isSelected ? undefined : o); | ||
}} | ||
> | ||
{o.startIcon && ( | ||
<SelectMenuIcon>{o.startIcon}</SelectMenuIcon> | ||
)} | ||
{o.label} | ||
</SelectMenuItem> | ||
); | ||
})} | ||
</SelectMenuList> | ||
) : ( | ||
<div | ||
css={(theme) => ({ | ||
display: "flex", | ||
alignItems: "center", | ||
justifyContent: "center", | ||
padding: 32, | ||
color: theme.palette.text.secondary, | ||
lineHeight: 1, | ||
})} | ||
> | ||
{emptyText ?? "No options found"} | ||
BrunoQuaresma marked this conversation as resolved.
Show resolved
Hide resolved
|
||
</div> | ||
) | ||
) : ( | ||
<Loader size={16} /> | ||
)} | ||
</SelectMenuContent> | ||
</SelectMenu> | ||
); | ||
}; | ||
|
||
export const SelectFilterSearch = SelectMenuSearch; |
Uh oh!
There was an error while loading. Please reload this page.