Apply review comments

coder · BrunoQuaresma · Jul 2, 2024 · Jun 25, 2024 · Jun 26, 2024 · Jun 26, 2024
commit 0d83bc8772ad743c577fdd84d38c76d1608786f5
diff --git a/site/src/components/Avatar/Avatar.tsx b/site/src/components/Avatar/Avatar.tsx
@@ -31,8 +31,7 @@ const sizeStyles = {
   xl: {
     width: 48,
     height: 48,
-    // Should never be overrided
-    fontSize: "24px !important",
+    fontSize: 24,
   },
 } satisfies Record<string, Interpolation<Theme>>;
 

diff --git a/site/src/components/Filter/SelectFilter.tsx b/site/src/components/Filter/SelectFilter.tsx
@@ -1,4 +1,3 @@
-import visuallyHidden from "@mui/utils/visuallyHidden";
 import { useState, type FC, type ReactNode } from "react";
 import { Loader } from "components/Loader/Loader";
 import {
@@ -32,7 +31,7 @@ export type SelectFilterProps = {
   emptyText?: string;
   onSelect: (option: SelectFilterOption | undefined) => void;
   // SelectFilterSearch element
-  search?: ReactNode;
+  selectFilterSearch?: ReactNode;
 };
 
 export const SelectFilter: FC<SelectFilterProps> = ({
@@ -42,7 +41,7 @@ export const SelectFilter: FC<SelectFilterProps> = ({
   onSelect,
   placeholder,
   emptyText,
-  search,
+  selectFilterSearch,
 }) => {
   const [open, setOpen] = useState(false);
 
@@ -52,24 +51,24 @@ export const SelectFilter: FC<SelectFilterProps> = ({
         <SelectMenuButton
           startIcon={selectedOption?.startIcon}
           css={{ width: BASE_WIDTH }}
+          aria-label={label}
         >
           {selectedOption?.label ?? placeholder}
-          <span css={{ ...visuallyHidden }}>{label}</span>
         </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,
+            // When including selectFilterSearch, we aim for the width to be as
+            // wide as possible.
+            width: selectFilterSearch ? "100%" : undefined,
             maxWidth: POPOVER_WIDTH,
             minWidth: BASE_WIDTH,
           },
         }}
       >
-        {search}
+        {selectFilterSearch}
         {options ? (
           options.length > 0 ? (
             <SelectMenuList>
@@ -103,7 +102,7 @@ export const SelectFilter: FC<SelectFilterProps> = ({
                 lineHeight: 1,
               })}
             >
-              {emptyText ?? "No options found"}
+              {emptyText || "No options found"}
             </div>
           )
         ) : (

diff --git a/site/src/components/SelectMenu/SelectMenu.stories.tsx b/site/src/components/SelectMenu/SelectMenu.stories.tsx
@@ -98,3 +98,35 @@ export const LongButtonText: Story = {
     );
   },
 };
+
+export const NoSelectedOption: Story = {
+  render: function SelectMenuRender() {
+    const opts = options(50);
+
+    return (
+      <SelectMenu>
+        <SelectMenuTrigger>
+          <SelectMenuButton css={{ width: 200 }}>All users</SelectMenuButton>
+        </SelectMenuTrigger>
+        <SelectMenuContent>
+          <SelectMenuSearch onChange={() => {}} />
+          <SelectMenuList>
+            {opts.map((o) => (
+              <SelectMenuItem key={o}>
+                <SelectMenuIcon>
+                  <UserAvatar size="xs" username={o} />
+                </SelectMenuIcon>
+                {o}
+              </SelectMenuItem>
+            ))}
+          </SelectMenuList>
+        </SelectMenuContent>
+      </SelectMenu>
+    );
+  },
+  play: async ({ canvasElement }) => {
+    const canvas = within(canvasElement);
+    const button = canvas.getByRole("button");
+    await userEvent.click(button);
+  },
+};
diff --git a/site/src/components/SelectMenu/SelectMenu.tsx b/site/src/components/SelectMenu/SelectMenu.tsx
@@ -8,6 +8,8 @@ import {
   Children,
   isValidElement,
   type HTMLProps,
+  type ReactElement,
+  useMemo,
 } from "react";
 import { DropdownArrow } from "components/DropdownArrow/DropdownArrow";
 import {
@@ -75,14 +77,13 @@ export const SelectMenuSearch: FC<SearchFieldProps> = (props) => {
       fullWidth
       size="medium"
       css={(theme) => ({
+        borderBottom: `1px solid ${theme.palette.divider}`,
         "& input": {
           fontSize: 14,
         },
-
         "& fieldset": {
           border: 0,
           borderRadius: 0,
-          borderBottom: `1px solid ${theme.palette.divider} !important`,
         },
         "& .MuiInputBase-root": {
           padding: `12px ${SIDE_PADDING}px`,
@@ -97,27 +98,35 @@ export const SelectMenuSearch: FC<SearchFieldProps> = (props) => {
 };
 
 export const SelectMenuList: FC<MenuListProps> = (props) => {
-  const items = Children.toArray(props.children);
-  type ItemType = (typeof items)[number];
-  const selectedAsFirst = (a: ItemType, b: ItemType) => {
-    if (
-      !isValidElement<MenuItemProps>(a) ||
-      !isValidElement<MenuItemProps>(b)
-    ) {
-      throw new Error(
-        "SelectMenuList children must be SelectMenuItem components",
-      );
+  const items = useMemo(() => {
+    let children = Children.toArray(props.children);
+    if (!children.every(isValidElement)) {
+      throw new Error("SelectMenuList only accepts MenuItem children");
     }
-    return a.props.selected ? -1 : 0;
-  };
-  items.sort(selectedAsFirst);
+    children = moveSelectedElementToFirst(
+      children as ReactElement<MenuItemProps>[],
+    );
+    return children;
+  }, [props.children]);
   return (
     <MenuList css={{ maxHeight: 480 }} {...props}>
       {items}
     </MenuList>
   );
 };
 
+function moveSelectedElementToFirst(items: ReactElement<MenuItemProps>[]) {
+  const selectedElement = items.find((i) => i.props.selected);
+  if (!selectedElement) {
+    return items;
+  }
+  const selectedElementIndex = items.indexOf(selectedElement);
+  const newItems = items.slice();
+  newItems.splice(selectedElementIndex, 1);
+  newItems.unshift(selectedElement);
+  return newItems;
+}
+
 export const SelectMenuIcon: FC<HTMLProps<HTMLDivElement>> = (props) => {
   return <div css={{ marginRight: 16 }} {...props} />;
 };