Popover and List improvments/bug fixes

package.json

@@ -40,7 +40,6 @@
     "node-addon-api": "^8.2.1",
     "polished": "^4.3.1",
     "sharp": "^0.33.5",
-    "solid-focus-trap": "^0.1.7",
     "tailwind-merge": "^2.5.3"
   },
   "devDependencies": {

src/renderer/src/components/dropdown-list/DropdownListItem.tsx

@@ -7,7 +7,7 @@ export type Props = JSX.IntrinsicElements["button"];
 const DropdownListItem: Component<Props> = (props) => {
   const state = useDropdownList();
   const value = state.namespace.create();
-  const { attrs, tabIndex, isSelected } = state.item(value, {
+  const { attrs, tabIndex } = state.item(value, {
     onPointerMove: state.handleItemPointerMove,
   });
 
@@ -19,10 +19,7 @@ const DropdownListItem: Component<Props> = (props) => {
     <RawList.Item
       onPointerLeave={state.handleItemPointerLeave}
       tabIndex={tabIndex()}
-      classList={{
-        "bg-overlay/30": state.isHighlighted() && isSelected(),
-      }}
-      {...attrs}
+      {...attrs()}
       {...props}
     />
   );

src/renderer/src/components/popover/Popover.tsx

@@ -30,6 +30,7 @@ import {
 } from "solid-js";
 
 export const DEFAULT_POPOVER_OPEN = false;
+export const DEFAULT_CUSTOM_POSITION_PLACEMENT: Placement = "bottom-start";
 
 export type Props = {
   offset?: OffsetOptions;
@@ -99,24 +100,6 @@ function useProviderValue(props: Props) {
     handleResize();
   };
 
-  let lastMousePos: [number, number];
-  const useCustomCoords = {
-    name: "useCustomCoords",
-    fn() {
-      const position = props.position?.();
-      if (position === undefined || position === lastMousePos) {
-        return {};
-      }
-
-      const [x, y] = position;
-      if (x === 0 || y === 0) {
-        return {};
-      }
-
-      return { x, y };
-    },
-  };
-
   const handleResize = () => {
     const trigger = triggerRef();
     const content = contentRef();
@@ -125,9 +108,12 @@ function useProviderValue(props: Props) {
       return;
     }
 
-    const middleware: Middleware[] = [offset(props.offset)];
-    if (typeof props.position !== "undefined") {
-      middleware.push(useCustomCoords);
+    const [x, y] = props.position?.() ?? [];
+    const hasCustomPosition = x !== undefined && y !== undefined;
+
+    const middleware: Middleware[] = [];
+    if (!hasCustomPosition) {
+      middleware.push(offset(props.offset));
     }
     if (typeof props.flip !== "undefined") {
       middleware.push(flip(props.flip === true ? undefined : props.flip));
@@ -136,10 +122,35 @@ function useProviderValue(props: Props) {
       middleware.push(shift(props.shift === true ? undefined : props.shift));
     }
 
-    computePosition(trigger, content, {
-      placement: props.placement,
-      strategy: "fixed",
+    const getBoundingClientRect = () => {
+      const triggerRect = trigger.getBoundingClientRect();
+      if (!hasCustomPosition) {
+        return triggerRect;
+      }
+
+      return {
+        x,
+        y,
+        top: y,
+        right: x,
+        bottom: y,
+        left: x,
+        width: 0,
+        height: 0,
+      };
+    };
+
+    const getPlacement = () => {
+      if (hasCustomPosition) {
+        return DEFAULT_CUSTOM_POSITION_PLACEMENT;
+      }
+      return props.placement;
+    };
+
+    computePosition({ getBoundingClientRect }, content, {
       middleware,
+      strategy: "fixed",
+      placement: getPlacement(),
     }).then(setPosition);
   };
 

src/renderer/src/components/popover/PopoverContent.tsx

@@ -1,7 +1,7 @@
 import { popoverStack, usePopover } from "./Popover";
 import { ComputePositionReturn } from "@floating-ui/dom";
 import { cn, sn } from "@renderer/lib/css.utils";
-import createFocusTrap from "solid-focus-trap";
+import createFocusTrap from "@renderer/lib/focus-trap";
 import { Component, createMemo, onCleanup, onMount, Show } from "solid-js";
 import { JSX } from "solid-js/jsx-runtime";
 
@@ -20,18 +20,18 @@ export type Props = JSX.IntrinsicElements["div"];
 const PopoverContent: Component<Props> = (props) => {
   const state = usePopover();
 
-  const isLastPopoupOpen = createMemo(() => {
+  const isLastPopupOpen = createMemo(() => {
     return state.isOpen() && popoverStack().at(-1) === state.id();
   });
 
   createFocusTrap({
     element: state.contentRef,
-    enabled: isLastPopoupOpen,
+    enabled: () => isLastPopupOpen(),
   });
 
   onMount(() => {
     const handleKeyUp = (e: KeyboardEvent) => {
-      if (!isLastPopoupOpen()) {
+      if (!isLastPopupOpen()) {
         return;
       }
 

src/renderer/src/components/popover/PopoverTrigger.tsx

@@ -1,20 +1,36 @@
 import { usePopover } from "./Popover";
-import { Component } from "solid-js";
+import { Component, splitProps, Switch, Match } from "solid-js";
 import { JSX } from "solid-js/jsx-runtime";
 
-export type Props = JSX.IntrinsicElements["button"];
-const PopoverTrigger: Component<Props> = (props) => {
+type PartialButtonProps = Partial<JSX.IntrinsicElements["button"]>;
+export type Props = Omit<PartialButtonProps, "ref" | "onClick" | "children"> & {
+  children?: JSX.Element | ((props: PartialButtonProps) => JSX.Element);
+};
+
+const PopoverTrigger: Component<Props> = (_props) => {
   const state = usePopover();
+  const [props, rest] = splitProps(_props, ["children"]);
+
+  const triggerProps: PartialButtonProps = {
+    ref: state.setTriggerRef,
+    // it complains about "data-open" not being a string for some reason
+    // so we have to cast it to a string
+    ["data-open" as string]: state.isOpen(),
+    onClick: () => {
+      state?.open();
+    },
+    ...rest,
+  };
 
   return (
-    <button
-      ref={state.setTriggerRef}
-      data-open={state.isOpen()}
-      onClick={() => {
-        state?.toggle();
-      }}
-      {...props}
-    />
+    <Switch>
+      <Match when={typeof props.children === "function"}>
+        {(props.children as (props: PartialButtonProps) => JSX.Element)(triggerProps)}
+      </Match>
+      <Match when={typeof props.children !== "function"}>
+        <button {...triggerProps}>{props.children as JSX.Element}</button>
+      </Match>
+    </Switch>
   );
 };
 

src/renderer/src/components/raw-list/RawList.tsx

@@ -10,7 +10,7 @@ export const RawListItem: Component<JSX.IntrinsicElements["button"]> = (_props)
   return (
     <button
       class={cn(
-        "flex items-center justify-between rounded-md px-2 py-1.5 focus:outline-none disabled:pointer-events-none disabled:opacity-50",
+        "data-[focused-keyboard=true]:focus:outline-initial flex items-center justify-between rounded-md px-2 py-1.5 disabled:pointer-events-none disabled:opacity-50 data-[focused=true]:bg-overlay/30 data-[focused-mouse=true]:focus:outline-none",
         props.class,
       )}
       {...rest}

src/renderer/src/components/selectable-list/SelectableListItem.tsx

@@ -12,11 +12,7 @@ const SelectableListItem: Component<Props> = (_props) => {
   const [props, rest] = splitProps(_props, ["value", "onSelectedByClick", "children"]);
 
   const state = useSelectableList();
-  const {
-    attrs,
-    isSelected: isFocused,
-    tabIndex,
-  } = state.item(props.value, {
+  const { attrs, tabIndex } = state.item(props.value, {
     onSelectedByClick: () => {
       props.onSelectedByClick?.();
       if (!props.value) {
@@ -32,14 +28,7 @@ const SelectableListItem: Component<Props> = (_props) => {
   });
 
   return (
-    <RawList.Item
-      tabIndex={tabIndex()}
-      classList={{
-        "bg-overlay/30": isFocused(),
-      }}
-      {...attrs}
-      {...rest}
-    >
+    <RawList.Item tabIndex={tabIndex()} {...attrs()} {...rest}>
       {props.children}
       <Show when={isSelected()}>
         <CheckIcon size={14} />

src/renderer/src/components/song/song-detail/SongControls.tsx

@@ -13,6 +13,7 @@ import {
   handleMuteSong,
 } from "../song.utils";
 import Button from "@renderer/components/button/Button";
+import SelectableList from "@renderer/components/selectable-list/SelectableList";
 import Slider from "@renderer/components/slider/Slider";
 import {
   CirclePlusIcon,
@@ -28,7 +29,6 @@ import {
   VolumeXIcon,
 } from "lucide-solid";
 import { Component, createMemo, createSignal, Match, Show, Switch, For } from "solid-js";
-import { ParentComponent } from "solid-js";
 import { Portal } from "solid-js/web";
 
 // Add a prop to accept the averageColor
@@ -190,6 +190,7 @@ const MIN_SPEED_AMOUNT = PREDEFINED_SPEEDS[0];
 const MAX_SPEED_AMOUNT = PREDEFINED_SPEEDS.at(-1);
 const RightPart = () => {
   const [isPopoverOpen, setisPopoverOpen] = createSignal(false);
+  const stringSpeed = createMemo(() => String(speed()));
 
   return (
     <div class="flex flex-1 justify-end gap-4">
@@ -201,16 +202,13 @@ const RightPart = () => {
           mainAxis: 10,
         }}
       >
-        <Popover.Anchor>
-          <Button
-            onClick={() => setisPopoverOpen(true)}
-            size="icon"
-            variant="ghost"
-            title="Set speed"
-          >
-            <GaugeIcon size={20} />
-          </Button>
-        </Popover.Anchor>
+        <Popover.Trigger>
+          {(triggerProps) => (
+            <Button {...triggerProps} size="icon" variant="ghost" title="Set speed">
+              <GaugeIcon size={20} />
+            </Button>
+          )}
+        </Popover.Trigger>
 
         <Portal>
           <Popover.Overlay />
@@ -234,9 +232,16 @@ const RightPart = () => {
             </div>
             <div class="my-2 h-px w-full bg-stroke" />
 
-            <For each={PREDEFINED_SPEEDS}>
-              {(amount) => <SpeedOption amount={amount}>{amount}</SpeedOption>}
-            </For>
+            <SelectableList
+              value={stringSpeed}
+              onValueChange={(newSpeed) => setSpeed(Number(newSpeed))}
+            >
+              <For each={PREDEFINED_SPEEDS}>
+                {(amount) => (
+                  <SelectableList.Item value={String(amount)}>{amount}</SelectableList.Item>
+                )}
+              </For>
+            </SelectableList>
           </Popover.Content>
         </Portal>
       </Popover>
@@ -247,18 +252,4 @@ const RightPart = () => {
   );
 };
 
-type SpeedOptionProps = {
-  amount: number;
-};
-const SpeedOption: ParentComponent<SpeedOptionProps> = (props) => {
-  return (
-    <button
-      onClick={() => setSpeed(props.amount)}
-      class="flex items-center justify-between rounded-md px-2 py-1.5 hover:bg-surface focus:outline-none disabled:pointer-events-none disabled:opacity-50"
-    >
-      {props.children}
-    </button>
-  );
-};
-
 export default SongControls;

src/renderer/src/components/song/song-list-search/SongListSearch.tsx

@@ -6,7 +6,7 @@ import { SongListSearchTags } from "./SongListSearchTags";
 import Button from "@renderer/components/button/Button";
 import { Input } from "@renderer/components/input/Input";
 import { FilterIcon, SearchIcon, FilterXIcon } from "lucide-solid";
-import { Accessor, Component, createSignal, Match, Setter, Signal, Switch } from "solid-js";
+import { Accessor, Component, createSignal, Match, Setter, Signal, Switch, Show } from "solid-js";
 
 export type SearchProps = {
   tags: Signal<Tag[]>;
@@ -109,8 +109,10 @@ const SongListSearch: Component<SearchProps> = (props) => {
         }}
       >
         <div class="mt-2 flex flex-nowrap items-center gap-2 overflow-y-auto">
-          <SongListSearchOrderBy disabled={!filterExpanded()} setOrder={props.setOrder} />
-          <SongListSearchTags disabled={!filterExpanded()} />
+          <Show when={filterExpanded()}>
+            <SongListSearchOrderBy setOrder={props.setOrder} />
+            <SongListSearchTags />
+          </Show>
         </div>
       </div>
     </div>

src/renderer/src/components/tabs/TabsTrigger.tsx

@@ -16,7 +16,7 @@ const TabsTrigger: Component<Props> = (_props) => {
     <button
       data-selected={isSelected()}
       tabIndex={tabIndex()}
-      {...attrs}
+      {...attrs()}
       {...rest}
       class={cn(
         "ring-offset-background z-10 flex h-[33px] items-center gap-2 rounded-lg px-3 text-subtext transition-colors",