app: prevent Claude model picker scroll jumps (#18041)

2026-10-21 19:11:17 +00:00 · 2026-08-26 14:15:56 -04:00 · 2026-08-26 14:15:56 -04:00 · a773b7affa
commit a773b7affa
parent d3a8af4d9b
2 changed files with 205 additions and 103 deletions
--- a/app/ui/app/src/components/ClaudeDesktopModelsSettings.interaction.test.tsx
+++ b/app/ui/app/src/components/ClaudeDesktopModelsSettings.interaction.test.tsx
@ -4,7 +4,13 @@ import {
  type ReactTestInstance,
  type ReactTestRenderer,
 } from "react-test-renderer";
-import { createRef } from "react";
+import {
+  createRef,
+  type ButtonHTMLAttributes,
+  type HTMLAttributes,
+  type MouseEvent as ReactMouseEvent,
+  type ReactNode,
+} from "react";
 import { describe, expect, it, vi } from "vitest";
 import { Switch } from "./ui/switch";
 import {
@ -12,6 +18,82 @@ import {
  type ClaudeDesktopModelsSettingsHandle,
 } from "./ClaudeDesktopModelsSettings";

+vi.mock("@headlessui/react", async (importOriginal) => {
+  const React = await import("react");
+  const original = await importOriginal<typeof import("@headlessui/react")>();
+  type PopoverContextValue = {
+    open: boolean;
+    close: () => void;
+    toggle: () => void;
+  };
+  const PopoverContext = React.createContext<PopoverContextValue | null>(null);
+  const usePopover = () => {
+    const context = React.useContext(PopoverContext);
+    if (!context) throw new Error("Popover components must be nested");
+    return context;
+  };
+
+  function TestPopover({
+    children,
+    className,
+  }: {
+    children: ReactNode;
+    className?: string;
+  }) {
+    const [open, setOpen] = React.useState(false);
+    const context = {
+      open,
+      close: () => setOpen(false),
+      toggle: () => setOpen((current) => !current),
+    };
+    return (
+      <PopoverContext.Provider value={context}>
+        <div className={className}>{children}</div>
+      </PopoverContext.Provider>
+    );
+  }
+
+  function TestPopoverButton({
+    onClick,
+    ...props
+  }: ButtonHTMLAttributes<HTMLButtonElement>) {
+    const { open, toggle } = usePopover();
+    return (
+      <button
+        {...props}
+        aria-expanded={open}
+        onClick={(event: ReactMouseEvent<HTMLButtonElement>) => {
+          onClick?.(event);
+          toggle();
+        }}
+      />
+    );
+  }
+
+  function TestPopoverPanel({
+    anchor,
+    children,
+    ...props
+  }: HTMLAttributes<HTMLDivElement> & {
+    anchor?: unknown;
+    children: ReactNode | ((props: { close: () => void }) => ReactNode);
+  }) {
+    const { open, close } = usePopover();
+    if (!open) return null;
+    return (
+      <div {...props} data-anchor={JSON.stringify(anchor)}>
+        {typeof children === "function" ? children({ close }) : children}
+      </div>
+    );
+  }
+
+  return Object.assign({}, original, {
+    Popover: TestPopover,
+    PopoverButton: TestPopoverButton,
+    PopoverPanel: TestPopoverPanel,
+  });
+});
+
 const fableRoute = {
  routeId: "claude-fable-5",
  routeName: "Fable 5",
@ -50,9 +132,7 @@ function testStatus(model = "glm-5.2:cloud", running = false) {

 async function selectKimi(renderer: ReactTestRenderer) {
  await act(async () => {
-    renderer.root
-      .findByProps({ "aria-label": "Ollama model for Fable 5" })
-      .props.onClick();
+    pickerButton(renderer).props.onClick();
    await Promise.resolve();
  });
  await act(async () => {
@ -61,6 +141,17 @@ async function selectKimi(renderer: ReactTestRenderer) {
  });
 }

+function pickerButton(renderer: ReactTestRenderer) {
+  const button = renderer.root
+    .findAllByType("button")
+    .find(
+      (candidate) =>
+        candidate.props["aria-label"] === "Ollama model for Fable 5",
+    );
+  if (!button) throw new Error("Claude model picker button not found");
+  return button;
+}
+
 function actionButton(renderer: ReactTestRenderer) {
  const button = renderer.root
    .findAllByType("button")
@ -80,10 +171,11 @@ function textContent(node: ReactTestInstance): string {
 }

 describe("ClaudeDesktopModelsSettings interactions", () => {
-  it("disables auto mode while model changes are not applied", async () => {
+  it("opens below without scrolling and disables auto mode for draft changes", async () => {
    class TestHTMLElement {
      focus() {}
    }
+    const focus = vi.fn();
    vi.stubGlobal("window", {
      addEventListener: vi.fn(),
      removeEventListener: vi.fn(),
@ -136,6 +228,10 @@ describe("ClaudeDesktopModelsSettings interactions", () => {
              ],
            }}
          />,
+          {
+            createNodeMock: (element) =>
+              element.type === "input" ? { focus } : null,
+          },
        );
        await Promise.resolve();
      });
@ -146,11 +242,19 @@ describe("ClaudeDesktopModelsSettings interactions", () => {
      expect(autoModeSwitch().props["aria-checked"]).toBe(true);

      await act(async () => {
-        renderer!.root
-          .findByProps({ "aria-label": "Ollama model for Fable 5" })
-          .props.onClick();
+        pickerButton(renderer!).props.onClick();
        await Promise.resolve();
      });
+      expect(
+        renderer!.root.findByProps({
+          "data-anchor": JSON.stringify({
+            to: "bottom end",
+            gap: 8,
+            padding: 8,
+          }),
+        }),
+      ).toBeDefined();
+      expect(focus).toHaveBeenCalledWith({ preventScroll: true });
      await act(async () => {
        const options = renderer!.root.findAllByProps({ role: "option" });
        options[1].props.onClick();
--- a/app/ui/app/src/components/ClaudeDesktopModelsSettings.tsx
+++ b/app/ui/app/src/components/ClaudeDesktopModelsSettings.tsx
@ -17,6 +17,7 @@ import {
  ChevronUpDownIcon,
  MagnifyingGlassIcon,
 } from "@heroicons/react/20/solid";
+import { Popover, PopoverButton, PopoverPanel } from "@headlessui/react";
 import {
  forwardRef,
  useCallback,
@ -145,54 +146,13 @@ function ClaudeModelPicker({
  disabled,
  onChange,
 }: ClaudeModelPickerProps) {
-  const [open, setOpen] = useState(false);
-  const [query, setQuery] = useState("");
-  const pickerRef = useRef<HTMLDivElement>(null);
-  const searchRef = useRef<HTMLInputElement>(null);
-  const normalizedQuery = query.trim().toLowerCase();
-  const filteredModels = models.filter((model) =>
-    model.displayName.toLowerCase().includes(normalizedQuery),
-  );
-
-  useEffect(() => {
-    if (!open) {
-      setQuery("");
-      return;
-    }
-    searchRef.current?.focus();
-
-    const handlePointerDown = (event: MouseEvent) => {
-      if (!pickerRef.current?.contains(event.target as Node)) setOpen(false);
-    };
-    const handleKeyDown = (event: KeyboardEvent) => {
-      if (event.key === "Escape") {
-        event.preventDefault();
-        setOpen(false);
-      }
-    };
-    document.addEventListener("mousedown", handlePointerDown);
-    document.addEventListener("keydown", handleKeyDown);
-    return () => {
-      document.removeEventListener("mousedown", handlePointerDown);
-      document.removeEventListener("keydown", handleKeyDown);
-    };
-  }, [open]);
-
-  const choose = (model: string) => {
-    onChange(model);
-    setOpen(false);
-  };
-
  return (
-    <div ref={pickerRef} className="relative min-w-0">
-      <button
+    <Popover className="relative min-w-0">
+      <PopoverButton
        id={id}
-        type="button"
        aria-label={`Ollama model for ${routeName}`}
        aria-haspopup="listbox"
-        aria-expanded={open}
        disabled={disabled}
-        onClick={() => setOpen((current) => !current)}
        className="flex min-h-9 w-full items-center gap-2 rounded-lg bg-neutral-50 px-3 py-1.5 text-left text-sm text-neutral-800 outline-none ring-1 ring-inset ring-neutral-200 hover:bg-neutral-100 focus:ring-2 focus:ring-blue-500 disabled:cursor-not-allowed disabled:opacity-50 dark:bg-neutral-700 dark:text-neutral-100 dark:ring-neutral-600 dark:hover:bg-neutral-600"
      >
        <span
@ -201,60 +161,98 @@ function ClaudeModelPicker({
          {value || "Select a model"}
        </span>
        <ChevronUpDownIcon className="h-4 w-4 flex-shrink-0 text-neutral-400" />
-      </button>
+      </PopoverButton>

-      {open && (
-        <div className="absolute bottom-full right-0 z-50 mb-2 w-full min-w-64 overflow-hidden rounded-2xl border border-neutral-100 bg-white text-[15px] text-neutral-800 shadow-xl shadow-black/5 dark:border-neutral-600/40 dark:bg-neutral-800 dark:text-white">
-          <div className="flex items-center gap-2 border-b border-neutral-100 px-3 py-2 dark:border-neutral-700">
-            <MagnifyingGlassIcon className="h-4 w-4 flex-shrink-0 text-neutral-400" />
-            <input
-              ref={searchRef}
-              type="text"
-              value={query}
-              onChange={(event) => setQuery(event.target.value)}
-              placeholder="Find model..."
-              aria-label={`Find model for ${routeName}`}
-              autoCorrect="off"
-              autoComplete="off"
-              className="min-w-0 flex-1 border-none bg-transparent py-0.5 outline-none"
-            />
-          </div>
-          <div role="listbox" className="max-h-64 overflow-y-auto py-1">
-            {filteredModels.map((model) => {
-              const available = modelIsAvailable(model);
-              const statusLabel = claudeDesktopModelStatusLabel(model);
-              const selected = value === model.name;
-              return (
-                <button
-                  key={model.name}
-                  type="button"
-                  role="option"
-                  aria-selected={selected}
-                  disabled={!available}
-                  onClick={() => choose(model.name)}
-                  className="flex w-full cursor-pointer items-start gap-2 px-3 py-2 text-left hover:bg-neutral-100 focus:bg-neutral-100 focus:outline-none disabled:cursor-not-allowed disabled:opacity-45 dark:hover:bg-neutral-700/60 dark:focus:bg-neutral-700/60"
-                >
-                  <span className="mt-0.5 h-4 w-4 flex-shrink-0">
-                    {selected && <CheckIcon className="h-4 w-4" />}
+      <PopoverPanel
+        anchor={{ to: "bottom end", gap: 8, padding: 8 }}
+        className="z-50 flex w-[var(--button-width)] min-w-64 flex-col overflow-hidden rounded-2xl border border-neutral-100 bg-white text-[15px] text-neutral-800 shadow-xl shadow-black/5 [--anchor-max-height:19rem] dark:border-neutral-600/40 dark:bg-neutral-800 dark:text-white"
+      >
+        {({ close }) => (
+          <ClaudeModelPickerOptions
+            routeName={routeName}
+            value={value}
+            models={models}
+            onChange={(model) => {
+              onChange(model);
+              close();
+            }}
+          />
+        )}
+      </PopoverPanel>
+    </Popover>
+  );
+}
+
+function ClaudeModelPickerOptions({
+  routeName,
+  value,
+  models,
+  onChange,
+}: Pick<
+  ClaudeModelPickerProps,
+  "routeName" | "value" | "models" | "onChange"
+>) {
+  const [query, setQuery] = useState("");
+  const searchRef = useRef<HTMLInputElement>(null);
+  const normalizedQuery = query.trim().toLowerCase();
+  const filteredModels = models.filter((model) =>
+    model.displayName.toLowerCase().includes(normalizedQuery),
+  );
+
+  useEffect(() => {
+    searchRef.current?.focus({ preventScroll: true });
+  }, []);
+
+  return (
+    <>
+      <div className="flex flex-none items-center gap-2 border-b border-neutral-100 px-3 py-2 dark:border-neutral-700">
+        <MagnifyingGlassIcon className="h-4 w-4 flex-shrink-0 text-neutral-400" />
+        <input
+          ref={searchRef}
+          type="text"
+          value={query}
+          onChange={(event) => setQuery(event.target.value)}
+          placeholder="Find model..."
+          aria-label={`Find model for ${routeName}`}
+          autoCorrect="off"
+          autoComplete="off"
+          className="min-w-0 flex-1 border-none bg-transparent py-0.5 outline-none"
+        />
+      </div>
+      <div role="listbox" className="min-h-0 overflow-y-auto py-1">
+        {filteredModels.map((model) => {
+          const available = modelIsAvailable(model);
+          const statusLabel = claudeDesktopModelStatusLabel(model);
+          const selected = value === model.name;
+          return (
+            <button
+              key={model.name}
+              type="button"
+              role="option"
+              aria-selected={selected}
+              disabled={!available}
+              onClick={() => onChange(model.name)}
+              className="flex w-full cursor-pointer items-start gap-2 px-3 py-2 text-left hover:bg-neutral-100 focus:bg-neutral-100 focus:outline-none disabled:cursor-not-allowed disabled:opacity-45 dark:hover:bg-neutral-700/60 dark:focus:bg-neutral-700/60"
+            >
+              <span className="mt-0.5 h-4 w-4 flex-shrink-0">
+                {selected && <CheckIcon className="h-4 w-4" />}
+              </span>
+              <span className="min-w-0 flex-1">
+                <span className="block truncate">{model.displayName}</span>
+                {statusLabel && (
+                  <span className="mt-0.5 block truncate text-xs text-neutral-400">
+                    {statusLabel}
                  </span>
-                  <span className="min-w-0 flex-1">
-                    <span className="block truncate">{model.displayName}</span>
-                    {statusLabel && (
-                      <span className="mt-0.5 block truncate text-xs text-neutral-400">
-                        {statusLabel}
-                      </span>
-                    )}
-                  </span>
-                </button>
-              );
-            })}
-            {filteredModels.length === 0 && (
-              <p className="px-3 py-2 text-neutral-400">No models found</p>
-            )}
-          </div>
-        </div>
-      )}
-    </div>
+                )}
+              </span>
+            </button>
+          );
+        })}
+        {filteredModels.length === 0 && (
+          <p className="px-3 py-2 text-neutral-400">No models found</p>
+        )}
+      </div>
+    </>
  );
 }