Skip to content

Commit a804a40

Browse files
BrunoQuaresmaBrunoQuaresma
committed
Apply michaels review
1 parent d1b71b1 commit a804a40

14 files changed

+87
-59
lines changed

‎.vscode/settings.json

+1
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,7 @@
155155
"typesafe",
156156
"unconvert",
157157
"Untar",
158+
"upsert",
158159
"Userspace",
159160
"VMID",
160161
"walkthrough",

‎site/src/components/Menu/MenuCheck.tsx

+10-9
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,17 @@
11
import CheckOutlined from "@mui/icons-material/CheckOutlined";
22
import type { FC } from "react";
3+
import { MenuIcon } from "./MenuIcon";
34

45
export const MenuCheck: FC<{ isVisible: boolean }> = ({ isVisible }) => {
56
return (
6-
<CheckOutlined
7-
role="presentation"
8-
css={{
9-
width: 14,
10-
height: 14,
11-
visibility: isVisible ? "visible" : "hidden",
12-
marginLeft: "auto",
13-
}}
14-
/>
7+
<MenuIcon>
8+
<CheckOutlined
9+
role="presentation"
10+
css={{
11+
visibility: isVisible ? "visible" : "hidden",
12+
marginLeft: "auto",
13+
}}
14+
/>
15+
</MenuIcon>
1516
);
1617
};

‎site/src/components/Menu/MenuIcon.tsx

+22
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
import { css } from "@emotion/css";
2+
import {
3+
type FC,
4+
type PropsWithChildren,
5+
type ReactElement,
6+
cloneElement,
7+
} from "react";
8+
9+
type MenuIconProps = {
10+
size?: number;
11+
};
12+
13+
export const MenuIcon: FC<PropsWithChildren<MenuIconProps>> = ({
14+
children,
15+
size = 14,
16+
}) => {
17+
return cloneElement(children as ReactElement, {
18+
className: css({
19+
fontSize: size,
20+
}),
21+
});
22+
};

‎site/src/components/Menu/MenuSearch.tsx

+3-9
Original file line numberDiff line numberDiff line change
@@ -16,15 +16,9 @@ export const MenuSearch: FC<SearchFieldProps> = (props) => {
1616
if (e.key === "ArrowDown") {
1717
e.preventDefault();
1818

19-
const popoverContent =
20-
e.currentTarget.closest<HTMLDivElement>(".MuiPaper-root");
21-
22-
if (!popoverContent) {
23-
return;
24-
}
25-
26-
const firstMenuItem =
27-
popoverContent.querySelector<HTMLElement>("[role=menuitem]");
19+
const firstMenuItem = e.currentTarget
20+
.closest<HTMLDivElement>(".MuiPaper-root")
21+
?.querySelector<HTMLElement>("[role=menuitem]");
2822

2923
if (firstMenuItem) {
3024
firstMenuItem.focus();

‎site/src/components/Popover/Popover.tsx

+1-1
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ export const usePopover = () => {
8282
};
8383

8484
export const withPopover =
85-
<TProps extends object>(
85+
<TProps extends Record<string, unknown>>(
8686
Component: FC<TProps>,
8787
popoverProps?: Omit<PopoverProps, "children">,
8888
): FC<TProps> =>

‎site/src/components/Search/SearchField.tsx

+1-1
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ export const SearchField: FC<SearchFieldProps> = (props) => {
7373
}}
7474
value={value}
7575
onChange={(e) => {
76-
onChange(e.currentTarget.value);
76+
onChange(e.target.value);
7777
}}
7878
placeholder={textFieldProps.placeholder ?? "Search..."}
7979
fullWidth

‎site/src/pages/WorkspacesPage/WorkspaceSearch/PresetFilterMenu.stories.tsx

+1-1
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ const meta: Meta<typeof PresetFiltersMenu> = {
1010
export default meta;
1111
type Story = StoryObj<typeof PresetFiltersMenu>;
1212

13-
export const Close: Story = {};
13+
export const Closed: Story = {};
1414

1515
export const Open: Story = {
1616
play: async ({ canvasElement }) => {

‎site/src/pages/WorkspacesPage/WorkspaceSearch/PresetFiltersMenu.tsx

+11-1
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,24 @@
1+
import OpenInNew from "@mui/icons-material/OpenInNew";
12
import Button from "@mui/material/Button";
23
import Divider from "@mui/material/Divider";
34
import MenuItem from "@mui/material/MenuItem";
45
import MenuList from "@mui/material/MenuList";
56
import type { FC } from "react";
67
import { DropdownArrow } from "components/DropdownArrow/DropdownArrow";
8+
import { MenuIcon } from "components/Menu/MenuIcon";
79
import {
810
Popover,
911
PopoverContent,
1012
PopoverTrigger,
1113
} from "components/Popover/Popover";
1214
import { docs } from "utils/docs";
1315

14-
const options = [
16+
type PresetOption = Readonly<{
17+
label: string;
18+
value: string;
19+
}>;
20+
21+
const options: PresetOption[] = [
1522
{ label: "My workspaces", value: "owner:me" },
1623
{ label: "All workspaces", value: "" },
1724
{ label: "Running workspaces", value: "status:running" },
@@ -56,6 +63,9 @@ export const PresetFiltersMenu: FC<PresetFilterMenuProps> = ({ onSelect }) => {
5663
}}
5764
>
5865
Learn advanced filtering
66+
<MenuIcon>
67+
<OpenInNew aria-label="Open in new tab" />
68+
</MenuIcon>
5969
</MenuItem>
6070
</MenuList>
6171
</PopoverContent>

‎site/src/pages/WorkspacesPage/WorkspaceSearch/StautsMenu.stories.tsx renamed to ‎site/src/pages/WorkspacesPage/WorkspaceSearch/StatusMenu.stories.tsx

-3
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,6 @@ import { StatusMenu } from "./StatusMenu";
66
const meta: Meta<typeof StatusMenu> = {
77
title: "pages/WorkspacesPage/StatusMenu",
88
component: StatusMenu,
9-
args: {
10-
placeholder: "All statuses",
11-
},
129
};
1310

1411
export default meta;

‎site/src/pages/WorkspacesPage/WorkspaceSearch/StatusMenu.tsx

+8-1
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ const StatusIndicator: FC<StatusIndicatorProps> = ({ color }) => {
1919

2020
return (
2121
<div
22+
role="presentation"
2223
css={{
2324
width: 8,
2425
height: 8,
@@ -29,7 +30,13 @@ const StatusIndicator: FC<StatusIndicatorProps> = ({ color }) => {
2930
);
3031
};
3132

32-
const options = [
33+
type StatusOption = Readonly<{
34+
label: string;
35+
value: string;
36+
indicator: JSX.Element;
37+
}>;
38+
39+
const options: StatusOption[] = [
3340
{
3441
label: "Running",
3542
value: "running",

‎site/src/pages/WorkspacesPage/WorkspaceSearch/TemplateMenu.tsx

+18-15
Original file line numberDiff line numberDiff line change
@@ -26,23 +26,25 @@ export const TemplateMenu = withPopover<TemplateMenuProps>((props) => {
2626
const popover = usePopover();
2727
const { organizationId, selected, onSelect } = props;
2828
const [filter, setFilter] = useState("");
29-
const templateOptionsQuery = useQuery({
29+
const [isHovered, setIsHovered] = useState(false);
30+
const { data: options } = useQuery({
3031
...templates(organizationId),
31-
enabled: selected !== undefined || popover.isOpen,
32+
enabled: selected !== undefined || popover.isOpen || isHovered,
33+
select: (data) =>
34+
data
35+
.filter((t) => {
36+
const f = filter.toLowerCase();
37+
return (
38+
t.name?.toLowerCase().includes(f) ||
39+
t.display_name.toLowerCase().includes(f)
40+
);
41+
})
42+
.map((t) => ({
43+
label: t.display_name ?? t.name,
44+
value: t.id,
45+
avatar: <TemplateAvatar size="xs" template={t} />,
46+
})),
3247
});
33-
const options = templateOptionsQuery.data
34-
?.filter((t) => {
35-
const f = filter.toLowerCase();
36-
return (
37-
t.name?.toLowerCase().includes(f) ||
38-
t.display_name.toLowerCase().includes(f)
39-
);
40-
})
41-
.map((t) => ({
42-
label: t.display_name ?? t.name,
43-
value: t.id,
44-
avatar: <TemplateAvatar size="xs" template={t} />,
45-
}));
4648
const selectedOption = options?.find((option) => option.value === selected);
4749

4850
return (
@@ -51,6 +53,7 @@ export const TemplateMenu = withPopover<TemplateMenuProps>((props) => {
5153
<MenuButton
5254
aria-label="Select template"
5355
startIcon={<span>{selectedOption?.avatar}</span>}
56+
onMouseEnter={() => setIsHovered(true)}
5457
>
5558
{selectedOption ? selectedOption.label : "All templates"}
5659
</MenuButton>

‎site/src/pages/WorkspacesPage/WorkspaceSearch/UserMenu.tsx

+9-8
Original file line numberDiff line numberDiff line change
@@ -36,10 +36,11 @@ export const UserMenu = withPopover<UserMenuProps>((props) => {
3636
const popover = usePopover();
3737
const { selected, onSelect } = props;
3838
const [filter, setFilter] = useState("");
39+
const [isHovered, setIsHovered] = useState(false);
3940
const debouncedFilter = useDebouncedValue(filter, 300);
4041
const usersQueryResult = useQuery({
4142
...usersQuery({ limit: 100, q: debouncedFilter }),
42-
enabled: popover.isOpen,
43+
enabled: popover.isOpen || isHovered,
4344
});
4445
const { data: selectedUser } = useQuery({
4546
queryKey: selectedUserKey(selected ?? ""),
@@ -57,6 +58,7 @@ export const UserMenu = withPopover<UserMenuProps>((props) => {
5758
<MenuButton
5859
aria-label="Select user"
5960
startIcon={<span>{selectedOption?.avatar}</span>}
61+
onMouseEnter={() => setIsHovered(true)}
6062
>
6163
{selectedOption ? selectedOption.label : "All users"}
6264
</MenuButton>
@@ -92,7 +94,10 @@ export const UserMenu = withPopover<UserMenuProps>((props) => {
9294

9395
// This avoid the need to refetch the selected user query
9496
// when the user is selected
95-
setSelectedUserQueryData(user, queryClient);
97+
queryClient.setQueryData(
98+
selectedUserKey(user.email),
99+
user,
100+
);
96101
popover.setIsOpen(false);
97102
onSelect(option.value);
98103
}}
@@ -133,10 +138,6 @@ async function getSelectedUser(
133138
return usersRes.users.at(0);
134139
}
135140

136-
function setSelectedUserQueryData(user: User, queryClient: QueryClient) {
137-
queryClient.setQueryData(selectedUserKey(user.email), user);
138-
}
139-
140141
function optionFromUser(user: User): UserOption {
141142
return {
142143
label: user.name ?? user.username,
@@ -150,12 +151,12 @@ function optionFromUser(user: User): UserOption {
150151
function mountOptions(
151152
users: readonly User[] | undefined,
152153
selectedUser: User | undefined,
153-
): UserOption[] | undefined {
154+
): Readonly<UserOption>[] | undefined {
154155
if (!users) {
155156
return undefined;
156157
}
157158

158-
let usersToDisplay = [...users];
159+
let usersToDisplay = users;
159160

160161
if (selectedUser) {
161162
const usersIncludeSelectedUser = users.some(

‎site/src/pages/WorkspacesPage/WorkspaceSearch/WorkspaceSearch.tsx

+2-6
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ export const WorkspaceSearch: FC<WorkspaceSearchProps> = ({
3838
<StatusMenu
3939
selected={status}
4040
onSelect={(status) => {
41-
setQuery(replaceOrAddTagValue(query, "status", status));
41+
setQuery(upsertTagValue(query, "status", status));
4242
}}
4343
/>
4444
</Stack>
@@ -56,11 +56,7 @@ function findTagValue(query: string, tag: string): string | undefined {
5656
return block.split(":")[1];
5757
}
5858

59-
function replaceOrAddTagValue(
60-
query: string,
61-
tag: string,
62-
value: string,
63-
): string {
59+
function upsertTagValue(query: string, tag: string, value: string): string {
6460
const blocks = query.split(" ");
6561
const block = blocks.find((block) => block.startsWith(`${tag}:`));
6662

‎site/src/theme/mui.ts

-4
Original file line numberDiff line numberDiff line change
@@ -305,10 +305,6 @@ export const components = {
305305
styleOverrides: {
306306
root: {
307307
gap: 12,
308-
309-
"& .MuiSvgIcon-root": {
310-
fontSize: 20,
311-
},
312308
},
313309
},
314310
},

0 commit comments

Comments
 (0)