Apply michaels review

coder · BrunoQuaresma · May 15, 2024 · May 15, 2024 · May 15, 2024 · May 15, 2024
commit a804a403e53675926d25dedabc210de6c7850bbf
diff --git a/.vscode/settings.json b/.vscode/settings.json
@@ -155,6 +155,7 @@
     "typesafe",
     "unconvert",
     "Untar",
+    "upsert",
     "Userspace",
     "VMID",
     "walkthrough",

diff --git a/site/src/components/Menu/MenuCheck.tsx b/site/src/components/Menu/MenuCheck.tsx
@@ -1,16 +1,17 @@
 import CheckOutlined from "@mui/icons-material/CheckOutlined";
 import type { FC } from "react";
+import { MenuIcon } from "./MenuIcon";
 
 export const MenuCheck: FC<{ isVisible: boolean }> = ({ isVisible }) => {
   return (
-    <CheckOutlined
-      role="presentation"
-      css={{
-        width: 14,
-        height: 14,
-        visibility: isVisible ? "visible" : "hidden",
-        marginLeft: "auto",
-      }}
-    />
+    <MenuIcon>
+      <CheckOutlined
+        role="presentation"
+        css={{
+          visibility: isVisible ? "visible" : "hidden",
+          marginLeft: "auto",
+        }}
+      />
+    </MenuIcon>
   );
 };
diff --git a/site/src/components/Menu/MenuIcon.tsx b/site/src/components/Menu/MenuIcon.tsx
@@ -0,0 +1,22 @@
+import { css } from "@emotion/css";
+import {
+  type FC,
+  type PropsWithChildren,
+  type ReactElement,
+  cloneElement,
+} from "react";
+
+type MenuIconProps = {
+  size?: number;
+};
+
+export const MenuIcon: FC<PropsWithChildren<MenuIconProps>> = ({
+  children,
+  size = 14,
+}) => {
+  return cloneElement(children as ReactElement, {
+    className: css({
+      fontSize: size,
+    }),
+  });
+};
diff --git a/site/src/components/Menu/MenuSearch.tsx b/site/src/components/Menu/MenuSearch.tsx
@@ -16,15 +16,9 @@ export const MenuSearch: FC<SearchFieldProps> = (props) => {
         if (e.key === "ArrowDown") {
           e.preventDefault();
 
-          const popoverContent =
-            e.currentTarget.closest<HTMLDivElement>(".MuiPaper-root");
-
-          if (!popoverContent) {
-            return;
-          }
-
-          const firstMenuItem =
-            popoverContent.querySelector<HTMLElement>("[role=menuitem]");
+          const firstMenuItem = e.currentTarget
+            .closest<HTMLDivElement>(".MuiPaper-root")
+            ?.querySelector<HTMLElement>("[role=menuitem]");
 
           if (firstMenuItem) {
             firstMenuItem.focus();

diff --git a/site/src/components/Popover/Popover.tsx b/site/src/components/Popover/Popover.tsx
@@ -82,7 +82,7 @@ export const usePopover = () => {
 };
 
 export const withPopover =
-  <TProps extends object>(
+  <TProps extends Record<string, unknown>>(
     Component: FC<TProps>,
     popoverProps?: Omit<PopoverProps, "children">,
   ): FC<TProps> =>

diff --git a/site/src/components/Search/SearchField.tsx b/site/src/components/Search/SearchField.tsx
@@ -73,7 +73,7 @@ export const SearchField: FC<SearchFieldProps> = (props) => {
         }}
         value={value}
         onChange={(e) => {
-          onChange(e.currentTarget.value);
+          onChange(e.target.value);
         }}
         placeholder={textFieldProps.placeholder ?? "Search..."}
         fullWidth

diff --git a/site/src/pages/WorkspacesPage/WorkspaceSearch/PresetFilterMenu.stories.tsx b/site/src/pages/WorkspacesPage/WorkspaceSearch/PresetFilterMenu.stories.tsx
@@ -10,7 +10,7 @@ const meta: Meta<typeof PresetFiltersMenu> = {
 export default meta;
 type Story = StoryObj<typeof PresetFiltersMenu>;
 
-export const Close: Story = {};
+export const Closed: Story = {};
 
 export const Open: Story = {
   play: async ({ canvasElement }) => {

diff --git a/site/src/pages/WorkspacesPage/WorkspaceSearch/PresetFiltersMenu.tsx b/site/src/pages/WorkspacesPage/WorkspaceSearch/PresetFiltersMenu.tsx
@@ -1,17 +1,24 @@
+import OpenInNew from "@mui/icons-material/OpenInNew";
 import Button from "@mui/material/Button";
 import Divider from "@mui/material/Divider";
 import MenuItem from "@mui/material/MenuItem";
 import MenuList from "@mui/material/MenuList";
 import type { FC } from "react";
 import { DropdownArrow } from "components/DropdownArrow/DropdownArrow";
+import { MenuIcon } from "components/Menu/MenuIcon";
 import {
   Popover,
   PopoverContent,
   PopoverTrigger,
 } from "components/Popover/Popover";
 import { docs } from "utils/docs";
 
-const options = [
+type PresetOption = Readonly<{
+  label: string;
+  value: string;
+}>;
+
+const options: PresetOption[] = [
   { label: "My workspaces", value: "owner:me" },
   { label: "All workspaces", value: "" },
   { label: "Running workspaces", value: "status:running" },
@@ -56,6 +63,9 @@ export const PresetFiltersMenu: FC<PresetFilterMenuProps> = ({ onSelect }) => {
                   }}
                 >
                   Learn advanced filtering
+                  <MenuIcon>
+                    <OpenInNew aria-label="Open in new tab" />
+                  </MenuIcon>
                 </MenuItem>
               </MenuList>
             </PopoverContent>

diff --git a/...ge/WorkspaceSearch/StautsMenu.stories.tsx → ...ge/WorkspaceSearch/StatusMenu.stories.tsx b/...ge/WorkspaceSearch/StautsMenu.stories.tsx → ...ge/WorkspaceSearch/StatusMenu.stories.tsx
@@ -6,9 +6,6 @@ import { StatusMenu } from "./StatusMenu";
 const meta: Meta<typeof StatusMenu> = {
   title: "pages/WorkspacesPage/StatusMenu",
   component: StatusMenu,
-  args: {
-    placeholder: "All statuses",
-  },
 };
 
 export default meta;

diff --git a/site/src/pages/WorkspacesPage/WorkspaceSearch/StatusMenu.tsx b/site/src/pages/WorkspacesPage/WorkspaceSearch/StatusMenu.tsx
@@ -19,6 +19,7 @@ const StatusIndicator: FC<StatusIndicatorProps> = ({ color }) => {
 
   return (
     <div
+      role="presentation"
       css={{
         width: 8,
         height: 8,
@@ -29,7 +30,13 @@ const StatusIndicator: FC<StatusIndicatorProps> = ({ color }) => {
   );
 };
 
-const options = [
+type StatusOption = Readonly<{
+  label: string;
+  value: string;
+  indicator: JSX.Element;
+}>;
+
+const options: StatusOption[] = [
   {
     label: "Running",
     value: "running",

diff --git a/site/src/pages/WorkspacesPage/WorkspaceSearch/TemplateMenu.tsx b/site/src/pages/WorkspacesPage/WorkspaceSearch/TemplateMenu.tsx
@@ -26,23 +26,25 @@ export const TemplateMenu = withPopover<TemplateMenuProps>((props) => {
   const popover = usePopover();
   const { organizationId, selected, onSelect } = props;
   const [filter, setFilter] = useState("");
-  const templateOptionsQuery = useQuery({
+  const [isHovered, setIsHovered] = useState(false);
+  const { data: options } = useQuery({
     ...templates(organizationId),
-    enabled: selected !== undefined || popover.isOpen,
+    enabled: selected !== undefined || popover.isOpen || isHovered,
+    select: (data) =>
+      data
+        .filter((t) => {
+          const f = filter.toLowerCase();
+          return (
+            t.name?.toLowerCase().includes(f) ||
+            t.display_name.toLowerCase().includes(f)
+          );
+        })
+        .map((t) => ({
+          label: t.display_name ?? t.name,
+          value: t.id,
+          avatar: <TemplateAvatar size="xs" template={t} />,
+        })),
   });
-  const options = templateOptionsQuery.data
-    ?.filter((t) => {
-      const f = filter.toLowerCase();
-      return (
-        t.name?.toLowerCase().includes(f) ||
-        t.display_name.toLowerCase().includes(f)
-      );
-    })
-    .map((t) => ({
-      label: t.display_name ?? t.name,
-      value: t.id,
-      avatar: <TemplateAvatar size="xs" template={t} />,
-    }));
   const selectedOption = options?.find((option) => option.value === selected);
 
   return (
@@ -51,6 +53,7 @@ export const TemplateMenu = withPopover<TemplateMenuProps>((props) => {
         <MenuButton
           aria-label="Select template"
           startIcon={<span>{selectedOption?.avatar}</span>}
+          onMouseEnter={() => setIsHovered(true)}
         >
           {selectedOption ? selectedOption.label : "All templates"}
         </MenuButton>

diff --git a/site/src/pages/WorkspacesPage/WorkspaceSearch/UserMenu.tsx b/site/src/pages/WorkspacesPage/WorkspaceSearch/UserMenu.tsx
@@ -36,10 +36,11 @@ export const UserMenu = withPopover<UserMenuProps>((props) => {
   const popover = usePopover();
   const { selected, onSelect } = props;
   const [filter, setFilter] = useState("");
+  const [isHovered, setIsHovered] = useState(false);
   const debouncedFilter = useDebouncedValue(filter, 300);
   const usersQueryResult = useQuery({
     ...usersQuery({ limit: 100, q: debouncedFilter }),
-    enabled: popover.isOpen,
+    enabled: popover.isOpen || isHovered,
   });
   const { data: selectedUser } = useQuery({
     queryKey: selectedUserKey(selected ?? ""),
@@ -57,6 +58,7 @@ export const UserMenu = withPopover<UserMenuProps>((props) => {
         <MenuButton
           aria-label="Select user"
           startIcon={<span>{selectedOption?.avatar}</span>}
+          onMouseEnter={() => setIsHovered(true)}
         >
           {selectedOption ? selectedOption.label : "All users"}
         </MenuButton>
@@ -92,7 +94,10 @@ export const UserMenu = withPopover<UserMenuProps>((props) => {
 
                       // This avoid the need to refetch the selected user query
                       // when the user is selected
-                      setSelectedUserQueryData(user, queryClient);
+                      queryClient.setQueryData(
+                        selectedUserKey(user.email),
+                        user,
+                      );
                       popover.setIsOpen(false);
                       onSelect(option.value);
                     }}
@@ -133,10 +138,6 @@ async function getSelectedUser(
   return usersRes.users.at(0);
 }
 
-function setSelectedUserQueryData(user: User, queryClient: QueryClient) {
-  queryClient.setQueryData(selectedUserKey(user.email), user);
-}
-
 function optionFromUser(user: User): UserOption {
   return {
     label: user.name ?? user.username,
@@ -150,12 +151,12 @@ function optionFromUser(user: User): UserOption {
 function mountOptions(
   users: readonly User[] | undefined,
   selectedUser: User | undefined,
-): UserOption[] | undefined {
+): Readonly<UserOption>[] | undefined {
   if (!users) {
     return undefined;
   }
 
-  let usersToDisplay = [...users];
+  let usersToDisplay = users;
 
   if (selectedUser) {
     const usersIncludeSelectedUser = users.some(

diff --git a/site/src/pages/WorkspacesPage/WorkspaceSearch/WorkspaceSearch.tsx b/site/src/pages/WorkspacesPage/WorkspaceSearch/WorkspaceSearch.tsx
@@ -38,7 +38,7 @@ export const WorkspaceSearch: FC<WorkspaceSearchProps> = ({
       <StatusMenu
         selected={status}
         onSelect={(status) => {
-          setQuery(replaceOrAddTagValue(query, "status", status));
+          setQuery(upsertTagValue(query, "status", status));
         }}
       />
     </Stack>
@@ -56,11 +56,7 @@ function findTagValue(query: string, tag: string): string | undefined {
   return block.split(":")[1];
 }
 
-function replaceOrAddTagValue(
-  query: string,
-  tag: string,
-  value: string,
-): string {
+function upsertTagValue(query: string, tag: string, value: string): string {
   const blocks = query.split(" ");
   const block = blocks.find((block) => block.startsWith(`${tag}:`));
 

diff --git a/site/src/theme/mui.ts b/site/src/theme/mui.ts
@@ -305,10 +305,6 @@ export const components = {
     styleOverrides: {
       root: {
         gap: 12,
-
-        "& .MuiSvgIcon-root": {
-          fontSize: 20,
-        },
       },
     },
   },