refactor(site): make minor design tweaks and fix issues on more options menus (#10493)

- Fix menus not closing when clicking and navigating to a lazy loaded page
- Minor design tweaks
- Make all "More options" menus consistent

Before:

<img width="243" alt="Screenshot 2023-11-02 at 10 21 02" src="https://github.com/coder/coder/assets/3165839/4d4eee7f-60d9-4c55-9559-468760715fe7">
<img width="246" alt="Screenshot 2023-11-02 at 10 18 03" src="https://github.com/coder/coder/assets/3165839/a834263a-f950-4f02-b3c7-c631928c0421">
<img width="251" alt="Screenshot 2023-11-02 at 10 07 40" src="https://github.com/coder/coder/assets/3165839/b2135281-1ffe-422b-a054-0c175f0dc2ad">

Now:

<img width="279" alt="Screenshot 2023-11-02 at 10 21 07" src="https://github.com/coder/coder/assets/3165839/a36b4025-3df0-4bd1-8071-7f1127caa2e2">
<img width="257" alt="Screenshot 2023-11-02 at 10 18 08" src="https://github.com/coder/coder/assets/3165839/57f737d4-fa32-4657-b59d-cf26029f8a69">
<img width="236" alt="Screenshot 2023-11-02 at 10 07 48" src="https://github.com/coder/coder/assets/3165839/a45a7f7d-f492-4498-a1f9-d86f7815d119">
This commit is contained in:
Bruno Quaresma
2023-11-02 21:32:04 -03:00
committed by GitHub
parent 2dce4151ba
commit 716b86b380
11 changed files with 279 additions and 272 deletions
+108
View File
@@ -0,0 +1,108 @@
import { useRef, useState, createContext, useContext, ReactNode } from "react";
import MoreVertOutlined from "@mui/icons-material/MoreVertOutlined";
import Menu, { MenuProps } from "@mui/material/Menu";
import MenuItem, { MenuItemProps } from "@mui/material/MenuItem";
import IconButton, { IconButtonProps } from "@mui/material/IconButton";
type MoreMenuContextValue = {
triggerRef: React.RefObject<HTMLButtonElement>;
close: () => void;
open: () => void;
isOpen: boolean;
};
const MoreMenuContext = createContext<MoreMenuContextValue | undefined>(
undefined,
);
export const MoreMenu = (props: { children: ReactNode }) => {
const triggerRef = useRef<HTMLButtonElement>(null);
const [isOpen, setIsOpen] = useState(false);
const close = () => {
setIsOpen(false);
};
const open = () => {
setIsOpen(true);
};
return (
<MoreMenuContext.Provider value={{ close, open, triggerRef, isOpen }}>
{props.children}
</MoreMenuContext.Provider>
);
};
const useMoreMenuContext = () => {
const ctx = useContext(MoreMenuContext);
if (!ctx) {
throw new Error("useMoreMenuContext must be used inside of MoreMenu");
}
return ctx;
};
export const MoreMenuTrigger = (props: IconButtonProps) => {
const menu = useMoreMenuContext();
return (
<IconButton
aria-controls="more-options"
aria-label="More options"
aria-haspopup="true"
onClick={menu.open}
ref={menu.triggerRef}
{...props}
>
<MoreVertOutlined />
</IconButton>
);
};
export const MoreMenuContent = (props: Omit<MenuProps, "open" | "onClose">) => {
const menu = useMoreMenuContext();
return (
<Menu
id="more-options"
anchorEl={menu.triggerRef.current}
open={menu.isOpen}
onClose={menu.close}
disablePortal
{...props}
/>
);
};
export const MoreMenuItem = (
props: MenuItemProps & { closeOnClick?: boolean; danger?: boolean },
) => {
const { closeOnClick = true, danger = false, ...menuItemProps } = props;
const ctx = useContext(MoreMenuContext);
if (!ctx) {
throw new Error("MoreMenuItem must be used inside of MoreMenu");
}
return (
<MenuItem
{...menuItemProps}
css={(theme) => ({
fontSize: 14,
color: danger ? theme.palette.error.light : undefined,
"& .MuiSvgIcon-root": {
width: theme.spacing(2),
height: theme.spacing(2),
},
})}
onClick={(e) => {
menuItemProps.onClick && menuItemProps.onClick(e);
if (closeOnClick) {
ctx.close();
}
}}
/>
);
};
@@ -1,24 +0,0 @@
import { TableRowMenu } from "./TableRowMenu";
import type { Meta, StoryObj } from "@storybook/react";
const meta: Meta<typeof TableRowMenu> = {
title: "components/TableRowMenu",
component: TableRowMenu,
};
export default meta;
type Story = StoryObj<typeof TableRowMenu<{ id: string }>>;
const Example: Story = {
args: {
data: { id: "123" },
menuItems: [
{ label: "Suspend", onClick: (data) => alert(data.id), disabled: false },
{ label: "Update", onClick: (data) => alert(data.id), disabled: false },
{ label: "Delete", onClick: (data) => alert(data.id), disabled: false },
{ label: "Explode", onClick: (data) => alert(data.id), disabled: true },
],
},
};
export { Example as TableRowMenu };
@@ -1,63 +0,0 @@
import IconButton from "@mui/material/IconButton";
import Menu, { MenuProps } from "@mui/material/Menu";
import MenuItem from "@mui/material/MenuItem";
import MoreVertIcon from "@mui/icons-material/MoreVert";
import { MouseEvent, useState } from "react";
export interface TableRowMenuProps<TData> {
data: TData;
menuItems: Array<{
label: React.ReactNode;
disabled: boolean;
onClick: (data: TData) => void;
}>;
}
export const TableRowMenu = <T,>({
data,
menuItems,
}: TableRowMenuProps<T>): JSX.Element => {
const [anchorEl, setAnchorEl] = useState<MenuProps["anchorEl"]>(null);
const handleClick = (event: MouseEvent) => {
setAnchorEl(event.currentTarget);
};
const handleClose = () => {
setAnchorEl(null);
};
return (
<>
<IconButton
size="small"
aria-label="more"
aria-controls="long-menu"
aria-haspopup="true"
onClick={handleClick}
>
<MoreVertIcon />
</IconButton>
<Menu
id="simple-menu"
anchorEl={anchorEl}
keepMounted
open={Boolean(anchorEl)}
onClose={handleClose}
>
{menuItems.map((item, index) => (
<MenuItem
key={index}
disabled={item.disabled}
onClick={() => {
handleClose();
item.onClick(data);
}}
>
{item.label}
</MenuItem>
))}
</Menu>
</>
);
};
+19 -12
View File
@@ -20,7 +20,6 @@ import {
PageHeaderTitle,
} from "components/PageHeader/PageHeader";
import { Stack } from "components/Stack/Stack";
import { TableRowMenu } from "components/TableRowMenu/TableRowMenu";
import { UserAutocomplete } from "components/UserAutocomplete/UserAutocomplete";
import { type FC, useState } from "react";
import { Helmet } from "react-helmet-async";
@@ -46,6 +45,12 @@ import Box from "@mui/material/Box";
import { LastSeen } from "components/LastSeen/LastSeen";
import { type Interpolation, type Theme } from "@emotion/react";
import LoadingButton from "@mui/lab/LoadingButton";
import {
MoreMenu,
MoreMenuContent,
MoreMenuItem,
MoreMenuTrigger,
} from "components/MoreMenu/MoreMenu";
export const GroupPage: FC = () => {
const { groupId } = useParams() as { groupId: string };
@@ -281,12 +286,12 @@ const GroupMemberRow = (props: {
</TableCell>
<TableCell width="1%">
{canUpdate && (
<TableRowMenu
data={member}
menuItems={[
{
label: "Remove",
onClick: async () => {
<MoreMenu>
<MoreMenuTrigger />
<MoreMenuContent>
<MoreMenuItem
danger
onClick={async () => {
try {
await removeMemberMutation.mutateAsync({
groupId: group.id,
@@ -298,11 +303,13 @@ const GroupMemberRow = (props: {
getErrorMessage(error, "Failed to remove member."),
);
}
},
disabled: group.id === group.organization_id,
},
]}
/>
}}
disabled={group.id === group.organization_id}
>
Remove
</MoreMenuItem>
</MoreMenuContent>
</MoreMenu>
)}
</TableCell>
</TableRow>
@@ -1,4 +1,4 @@
import { type FC, useRef, useState } from "react";
import { type FC } from "react";
import { Link as RouterLink, useNavigate } from "react-router-dom";
import { useDeletionDialogState } from "./useDeletionDialogState";
@@ -20,17 +20,19 @@ import {
PageHeaderTitle,
PageHeaderSubtitle,
} from "components/PageHeader/PageHeader";
import Button from "@mui/material/Button";
import MoreVertOutlined from "@mui/icons-material/MoreVertOutlined";
import Menu from "@mui/material/Menu";
import MenuItem from "@mui/material/MenuItem";
import IconButton from "@mui/material/IconButton";
import AddIcon from "@mui/icons-material/AddOutlined";
import SettingsIcon from "@mui/icons-material/SettingsOutlined";
import DeleteIcon from "@mui/icons-material/DeleteOutlined";
import EditIcon from "@mui/icons-material/EditOutlined";
import CopyIcon from "@mui/icons-material/FileCopyOutlined";
import {
MoreMenu,
MoreMenuContent,
MoreMenuItem,
MoreMenuTrigger,
} from "components/MoreMenu/MoreMenu";
import Divider from "@mui/material/Divider";
type TemplateMenuProps = {
templateName: string;
@@ -46,80 +48,54 @@ const TemplateMenu: FC<TemplateMenuProps> = ({
onDelete,
}) => {
const dialogState = useDeletionDialogState(templateId, onDelete);
const menuTriggerRef = useRef<HTMLButtonElement>(null);
const [isMenuOpen, setIsMenuOpen] = useState(false);
const navigate = useNavigate();
const queryText = `template:${templateName}`;
const workspaceCountQuery = useQuery({
...workspaces({ q: queryText }),
select: (res) => res.count,
});
// Returns a function that will execute the action and close the menu
const onMenuItemClick = (actionFn: () => void) => () => {
setIsMenuOpen(false);
actionFn();
};
const safeToDeleteTemplate = workspaceCountQuery.data === 0;
return (
<>
<div>
<IconButton
aria-controls="template-options"
aria-haspopup="true"
onClick={() => setIsMenuOpen(true)}
ref={menuTriggerRef}
arial-label="More options"
>
<MoreVertOutlined />
</IconButton>
<Menu
id="template-options"
anchorEl={menuTriggerRef.current}
open={isMenuOpen}
onClose={() => setIsMenuOpen(false)}
>
<MenuItem
onClick={onMenuItemClick(() =>
navigate(`/templates/${templateName}/settings`),
)}
<MoreMenu>
<MoreMenuTrigger />
<MoreMenuContent>
<MoreMenuItem
onClick={() => {
navigate(`/templates/${templateName}/settings`);
}}
>
<SettingsIcon />
Settings
</MenuItem>
</MoreMenuItem>
<MenuItem
onClick={onMenuItemClick(() =>
<MoreMenuItem
onClick={() => {
navigate(
`/templates/${templateName}/versions/${templateVersion}/edit`,
),
)}
);
}}
>
<EditIcon />
Edit files
</MenuItem>
</MoreMenuItem>
<MenuItem
onClick={onMenuItemClick(() =>
navigate(`/templates/new?fromTemplate=${templateName}`),
)}
<MoreMenuItem
onClick={() => {
navigate(`/templates/new?fromTemplate=${templateName}`);
}}
>
<CopyIcon />
Duplicate&hellip;
</MenuItem>
<MenuItem
onClick={onMenuItemClick(dialogState.openDeleteConfirmation)}
>
</MoreMenuItem>
<Divider />
<MoreMenuItem onClick={dialogState.openDeleteConfirmation} danger>
<DeleteIcon />
Delete&hellip;
</MenuItem>
</Menu>
</div>
</MoreMenuItem>
</MoreMenuContent>
</MoreMenu>
{safeToDeleteTemplate ? (
<DeleteDialog
@@ -20,7 +20,6 @@ import { ChooseOne, Cond } from "components/Conditionals/ChooseOne";
import { EmptyState } from "components/EmptyState/EmptyState";
import { Stack } from "components/Stack/Stack";
import { TableLoader } from "components/TableLoader/TableLoader";
import { TableRowMenu } from "components/TableRowMenu/TableRowMenu";
import {
UserOrGroupAutocomplete,
UserOrGroupAutocompleteValue,
@@ -30,6 +29,12 @@ import { GroupAvatar } from "components/GroupAvatar/GroupAvatar";
import { getGroupSubtitle } from "utils/groups";
import { PageHeader, PageHeaderTitle } from "components/PageHeader/PageHeader";
import LoadingButton from "@mui/lab/LoadingButton";
import {
MoreMenu,
MoreMenuContent,
MoreMenuItem,
MoreMenuTrigger,
} from "components/MoreMenu/MoreMenu";
type AddTemplateUserOrGroupProps = {
organizationId: string;
@@ -281,16 +286,17 @@ export const TemplatePermissionsPageView: FC<
<TableCell>
{canUpdatePermissions && (
<TableRowMenu
data={group}
menuItems={[
{
label: "Remove",
onClick: () => onRemoveGroup(group),
disabled: false,
},
]}
/>
<MoreMenu>
<MoreMenuTrigger />
<MoreMenuContent>
<MoreMenuItem
danger
onClick={() => onRemoveGroup(group)}
>
Remove
</MoreMenuItem>
</MoreMenuContent>
</MoreMenu>
)}
</TableCell>
</TableRow>
@@ -327,16 +333,17 @@ export const TemplatePermissionsPageView: FC<
<TableCell>
{canUpdatePermissions && (
<TableRowMenu
data={user}
menuItems={[
{
label: "Remove",
onClick: () => onRemoveUser(user),
disabled: false,
},
]}
/>
<MoreMenu>
<MoreMenuTrigger />
<MoreMenuContent>
<MoreMenuItem
danger
onClick={() => onRemoveUser(user)}
>
Remove
</MoreMenuItem>
</MoreMenuContent>
</MoreMenu>
)}
</TableCell>
</TableRow>
+11 -16
View File
@@ -21,12 +21,11 @@ const renderPage = () => {
const suspendUser = async () => {
const user = userEvent.setup();
// Get the first user in the table
const moreButtons = await screen.findAllByLabelText("more");
const moreButtons = await screen.findAllByLabelText("More options");
const firstMoreButton = moreButtons[0];
await user.click(firstMoreButton);
const menu = await screen.findByRole("menu");
const suspendButton = within(menu).getByText(/Suspend/);
const suspendButton = screen.getByTestId("suspend-button");
await user.click(suspendButton);
// Check if the confirm message is displayed
@@ -39,17 +38,15 @@ const suspendUser = async () => {
const deleteUser = async () => {
const user = userEvent.setup();
// Click on the "more" button to display the "Delete" option
// Click on the "More options" button to display the "Delete" option
// Needs to await fetching users and fetching permissions, because they're needed to see the more button
const moreButtons = await screen.findAllByLabelText("more");
const moreButtons = await screen.findAllByLabelText("More options");
// get MockUser2
const selectedMoreButton = moreButtons[1];
await user.click(selectedMoreButton);
const menu = await screen.findByRole("menu");
const deleteButton = within(menu).getByText(/Delete/);
const deleteButton = screen.getByText(/Delete/);
await user.click(deleteButton);
// Check if the confirm message is displayed
@@ -67,12 +64,11 @@ const deleteUser = async () => {
};
const activateUser = async () => {
const moreButtons = await screen.findAllByLabelText("more");
const moreButtons = await screen.findAllByLabelText("More options");
const suspendedMoreButton = moreButtons[2];
fireEvent.click(suspendedMoreButton);
const menu = screen.getByRole("menu");
const activateButton = within(menu).getByText(/Activate/);
const activateButton = screen.getByText(/Activate/);
fireEvent.click(activateButton);
// Check if the confirm message is displayed
@@ -86,14 +82,11 @@ const activateUser = async () => {
};
const resetUserPassword = async (setupActionSpies: () => void) => {
const moreButtons = await screen.findAllByLabelText("more");
const moreButtons = await screen.findAllByLabelText("More options");
const firstMoreButton = moreButtons[0];
fireEvent.click(firstMoreButton);
const menu = screen.getByRole("menu");
const resetPasswordButton = within(menu).getByText(/Reset password/);
const resetPasswordButton = screen.getByText(/Reset password/);
fireEvent.click(resetPasswordButton);
// Check if the confirm message is displayed
@@ -135,6 +128,8 @@ const updateUserRole = async (role: Role) => {
};
};
jest.spyOn(console, "error").mockImplementation(() => {});
describe("UsersPage", () => {
describe("suspend user", () => {
describe("when it is success", () => {
@@ -67,7 +67,7 @@ export const UsersTable: FC<React.PropsWithChildren<UsersTableProps>> = ({
}) => {
return (
<TableContainer>
<Table>
<Table data-testid="users-table">
<TableHead>
<TableRow>
<TableCell width="29%">{Language.usernameLabel}</TableCell>
@@ -15,7 +15,6 @@ import {
TableLoaderSkeleton,
TableRowSkeleton,
} from "components/TableLoader/TableLoader";
import { TableRowMenu } from "components/TableRowMenu/TableRowMenu";
import { EnterpriseBadge } from "components/DeploySettingsLayout/Badges";
import HideSourceOutlined from "@mui/icons-material/HideSourceOutlined";
import KeyOutlined from "@mui/icons-material/KeyOutlined";
@@ -26,6 +25,13 @@ import { LastSeen } from "components/LastSeen/LastSeen";
import { UserRoleCell } from "./UserRoleCell";
import { type GroupsByUserId } from "api/queries/groups";
import { UserGroupsCell } from "./UserGroupsCell";
import {
MoreMenu,
MoreMenuTrigger,
MoreMenuContent,
MoreMenuItem,
} from "components/MoreMenu/MoreMenu";
import Divider from "@mui/material/Divider";
dayjs.extend(relativeTime);
@@ -176,48 +182,49 @@ export const UsersTableBody: FC<
{canEditUsers && (
<TableCell>
<TableRowMenu
data={user}
menuItems={[
// Return either suspend or activate depending on status
user.status === "active" || user.status === "dormant"
? {
label: <>Suspend&hellip;</>,
onClick: onSuspendUser,
disabled: false,
}
: {
label: <>Activate&hellip;</>,
onClick: onActivateUser,
disabled: false,
},
{
label: <>Delete&hellip;</>,
onClick: onDeleteUser,
disabled: user.id === actorID,
},
{
label: <>Reset password&hellip;</>,
onClick: onResetUserPassword,
disabled: user.login_type !== "password",
},
{
label: "View workspaces",
onClick: onListWorkspaces,
disabled: false,
},
{
label: (
<>
View activity
{!canViewActivity && <EnterpriseBadge />}
</>
),
onClick: onViewActivity,
disabled: !canViewActivity,
},
]}
/>
<MoreMenu>
<MoreMenuTrigger />
<MoreMenuContent>
{user.status === "active" || user.status === "dormant" ? (
<MoreMenuItem
data-testid="suspend-button"
onClick={() => {
onSuspendUser(user);
}}
>
Suspend&hellip;
</MoreMenuItem>
) : (
<MoreMenuItem onClick={() => onActivateUser(user)}>
Activate&hellip;
</MoreMenuItem>
)}
<MoreMenuItem onClick={() => onListWorkspaces(user)}>
View workspaces
</MoreMenuItem>
<MoreMenuItem
onClick={() => onViewActivity(user)}
disabled={!canViewActivity}
>
View activity
{!canViewActivity && <EnterpriseBadge />}
</MoreMenuItem>
<MoreMenuItem
onClick={() => onResetUserPassword(user)}
disabled={user.login_type !== "password"}
>
Reset password&hellip;
</MoreMenuItem>
<Divider />
<MoreMenuItem
onClick={() => onDeleteUser(user)}
disabled={user.id === actorID}
danger
>
Delete&hellip;
</MoreMenuItem>
</MoreMenuContent>
</MoreMenu>
</TableCell>
)}
</TableRow>
@@ -1,8 +1,5 @@
import MenuItem from "@mui/material/MenuItem";
import Menu from "@mui/material/Menu";
import MoreVertOutlined from "@mui/icons-material/MoreVertOutlined";
import { type FC, Fragment, type ReactNode, useRef, useState } from "react";
import type { Workspace, WorkspaceBuildParameter } from "api/typesGenerated";
import { FC, Fragment, ReactNode } from "react";
import { Workspace, WorkspaceBuildParameter } from "api/typesGenerated";
import {
ActionLoadingButton,
CancelButton,
@@ -21,7 +18,13 @@ import {
import SettingsOutlined from "@mui/icons-material/SettingsOutlined";
import HistoryOutlined from "@mui/icons-material/HistoryOutlined";
import DeleteOutlined from "@mui/icons-material/DeleteOutlined";
import IconButton from "@mui/material/IconButton";
import {
MoreMenu,
MoreMenuContent,
MoreMenuItem,
MoreMenuTrigger,
} from "components/MoreMenu/MoreMenu";
import Divider from "@mui/material/Divider";
export interface WorkspaceActionsProps {
workspace: Workspace;
@@ -65,8 +68,6 @@ export const WorkspaceActions: FC<WorkspaceActionsProps> = ({
canChangeVersions,
);
const canBeUpdated = workspace.outdated && canAcceptJobs;
const menuTriggerRef = useRef<HTMLButtonElement>(null);
const [isMenuOpen, setIsMenuOpen] = useState(false);
// A mapping of button type to the corresponding React component
const buttonMapping: ButtonMapping = {
@@ -106,12 +107,6 @@ export const WorkspaceActions: FC<WorkspaceActionsProps> = ({
),
};
// Returns a function that will execute the action and close the menu
const onMenuItemClick = (actionFn: () => void) => () => {
setIsMenuOpen(false);
actionFn();
};
return (
<div
css={(theme) => ({
@@ -131,44 +126,36 @@ export const WorkspaceActions: FC<WorkspaceActionsProps> = ({
<Fragment key={action}>{buttonMapping[action]}</Fragment>
))}
{canCancel && <CancelButton handleAction={handleCancel} />}
<div>
<IconButton
<MoreMenu>
<MoreMenuTrigger
title="More options"
size="small"
data-testid="workspace-options-button"
aria-controls="workspace-options"
aria-haspopup="true"
disabled={!canAcceptJobs}
ref={menuTriggerRef}
onClick={() => setIsMenuOpen(true)}
>
<MoreVertOutlined />
</IconButton>
<Menu
id="workspace-options"
anchorEl={menuTriggerRef.current}
open={isMenuOpen}
onClose={() => setIsMenuOpen(false)}
>
<MenuItem onClick={onMenuItemClick(handleSettings)}>
/>
<MoreMenuContent id="workspace-options">
<MoreMenuItem onClick={handleSettings}>
<SettingsOutlined />
Settings
</MenuItem>
</MoreMenuItem>
{canChangeVersions && (
<MenuItem onClick={onMenuItemClick(handleChangeVersion)}>
<MoreMenuItem onClick={handleChangeVersion}>
<HistoryOutlined />
Change version&hellip;
</MenuItem>
</MoreMenuItem>
)}
<MenuItem
onClick={onMenuItemClick(handleDelete)}
<Divider />
<MoreMenuItem
danger
onClick={handleDelete}
data-testid="delete-button"
>
<DeleteOutlined />
Delete&hellip;
</MenuItem>
</Menu>
</div>
</MoreMenuItem>
</MoreMenuContent>
</MoreMenu>
</div>
);
};
+7
View File
@@ -342,6 +342,13 @@ dark = createTheme(dark, {
padding: "4px 0",
minWidth: 160,
},
root: {
// It should be the same as the menu padding
"& .MuiDivider-root": {
marginTop: 4,
marginBottom: 4,
},
},
},
},
MuiMenuItem: {