fix: address review feedback for terminal copy-on-select

Author: pallyoung

packages/web/src/app/providers.lifecycle.test.tsx

@@ -414,6 +414,99 @@ describe("AppProviders lifecycle recovery", () => {
     expect(sendCommand).toHaveBeenCalledWith("settings.get", {}, undefined);
   });
 
+  it("preserves a newer local terminal copy-on-select update when startup hydration resolves later", async () => {
+    const store = createStore();
+    setVisibilityState("visible");
+
+    let resolveSettingsGet: ((value: Record<string, unknown>) => void) | undefined;
+    const settingsGetPromise = new Promise<Record<string, unknown>>((resolve) => {
+      resolveSettingsGet = resolve;
+    });
+    const sendCommand = vi.fn().mockImplementation(async (op: string) => {
+      if (op === "settings.get") {
+        return await settingsGetPromise;
+      }
+
+      return undefined;
+    });
+    wsState.client!.sendCommand = sendCommand;
+
+    renderProviders(store);
+
+    await vi.waitFor(() => {
+      expect(wsState.client?.connect).toHaveBeenCalled();
+    });
+
+    act(() => {
+      wsState.client?.statusHandler?.("connected");
+    });
+
+    await vi.waitFor(() => {
+      expect(sendCommand).toHaveBeenCalledWith("settings.get", {}, undefined);
+    });
+
+    act(() => {
+      store.set(terminalPreferencesAtom, { copyOnSelect: true });
+    });
+
+    await act(async () => {
+      resolveSettingsGet?.({});
+      await settingsGetPromise;
+    });
+
+    expect(store.get(terminalPreferencesAtom)).toEqual({ copyOnSelect: true });
+  });
+
+  it("preserves an ABA local terminal copy-on-select update when startup hydration resolves later", async () => {
+    const store = createStore();
+    setVisibilityState("visible");
+
+    act(() => {
+      store.set(terminalPreferencesAtom, { copyOnSelect: true });
+    });
+
+    let resolveSettingsGet: ((value: Record<string, unknown>) => void) | undefined;
+    const settingsGetPromise = new Promise<Record<string, unknown>>((resolve) => {
+      resolveSettingsGet = resolve;
+    });
+    const sendCommand = vi.fn().mockImplementation(async (op: string) => {
+      if (op === "settings.get") {
+        return await settingsGetPromise;
+      }
+
+      return undefined;
+    });
+    wsState.client!.sendCommand = sendCommand;
+
+    renderProviders(store);
+
+    await vi.waitFor(() => {
+      expect(wsState.client?.connect).toHaveBeenCalled();
+    });
+
+    act(() => {
+      wsState.client?.statusHandler?.("connected");
+    });
+
+    await vi.waitFor(() => {
+      expect(sendCommand).toHaveBeenCalledWith("settings.get", {}, undefined);
+    });
+
+    act(() => {
+      store.set(terminalPreferencesAtom, { copyOnSelect: false });
+      store.set(terminalPreferencesAtom, { copyOnSelect: true });
+    });
+
+    await act(async () => {
+      resolveSettingsGet?.({
+        "appearance.terminalCopyOnSelect": false,
+      });
+      await settingsGetPromise;
+    });
+
+    expect(store.get(terminalPreferencesAtom)).toEqual({ copyOnSelect: true });
+  });
+
   it("marks the session authenticated when /auth/status confirms an existing server session", async () => {
     globalThis.fetch = vi.fn().mockResolvedValue({
       json: async () => ({ authEnabled: true, authenticated: true }),

packages/web/src/app/providers.tsx

@@ -208,13 +208,21 @@ export function AppProviders({ children }: AppProvidersProps) {
     }
 
     let cancelled = false;
+    let localTerminalPreferencesUpdated = false;
+    const unsubscribeTerminalPreferences = store.sub(terminalPreferencesAtom, () => {
+      localTerminalPreferencesUpdated = true;
+    });
 
     const hydrateTerminalPreferences = async () => {
       const result = await dispatch<Record<string, unknown>>("settings.get", {});
       if (cancelled || !result.ok || !result.data) {
         return;
       }
 
+      if (localTerminalPreferencesUpdated) {
+        return;
+      }
+
       setTerminalPreferences({
         copyOnSelect: resolveTerminalCopyOnSelectSetting(result.data),
       });
@@ -224,8 +232,9 @@ export function AppProviders({ children }: AppProvidersProps) {
 
     return () => {
       cancelled = true;
+      unsubscribeTerminalPreferences();
     };
-  }, [connectionStatus, dispatch, setTerminalPreferences]);
+  }, [connectionStatus, dispatch, setTerminalPreferences, store]);
 
   useEffect(() => {
     activeWorkspaceIdRef.current = activeWorkspaceId;

packages/web/src/features/settings/components/settings-page.test.tsx

@@ -905,6 +905,44 @@ describe("SettingsPage", () => {
     expect(chineseLanguagePill).toHaveAttribute("aria-pressed", "true");
   });
 
+  it("keeps copy-on-select visible on desktop appearance settings", async () => {
+    const sendCommand = vi.fn().mockImplementation(async (op: string) => {
+      if (op === "settings.get") {
+        return {
+          "appearance.terminalCopyOnSelect": true,
+        };
+      }
+      return {};
+    });
+    const store = createConnectedStore(sendCommand);
+
+    renderSettingsPage(store);
+    fireEvent.click(screen.getByRole("button", { name: "外观" }));
+
+    expect(await screen.findByRole("switch", { name: "选中自动复制" })).toBeInTheDocument();
+  });
+
+  it("does not show copy-on-select on mobile appearance settings", async () => {
+    viewportMocks.viewport = "mobile";
+    const sendCommand = vi.fn().mockImplementation(async (op: string) => {
+      if (op === "settings.get") {
+        return {
+          "appearance.terminalCopyOnSelect": true,
+        };
+      }
+      return {};
+    });
+    const store = createConnectedStore(sendCommand);
+
+    renderSettingsPage(store);
+    fireEvent.click(screen.getByRole("button", { name: "外观" }));
+
+    await screen.findByText("主题");
+
+    expect(screen.queryByRole("switch", { name: "选中自动复制" })).not.toBeInTheDocument();
+    expect(screen.queryByText("选中自动复制")).not.toBeInTheDocument();
+  });
+
   it("updates theme selection through the shared appearance pills", async () => {
     const sendCommand = vi.fn().mockImplementation(async (op: string) => {
       if (op === "settings.get") {

packages/web/src/features/settings/components/settings-page.tsx

@@ -331,6 +331,7 @@ export function SettingsPage() {
       case "appearance":
         return (
           <AppearanceSettings
+            isMobile={isMobile}
             locale={locale}
             setLocale={handleLocaleSelection}
             terminalRenderer={terminalRenderer}
@@ -753,6 +754,7 @@ function GeneralSettings({
 }
 
 interface AppearanceSettingsProps {
+  isMobile: boolean;
   locale: string;
   setLocale: (value: "zh" | "en") => void;
   terminalRenderer: "standard" | "compatibility";
@@ -764,6 +766,7 @@ interface AppearanceSettingsProps {
 }
 
 function AppearanceSettings({
+  isMobile,
   locale,
   setLocale,
   terminalRenderer,
@@ -863,26 +866,28 @@ function AppearanceSettings({
           </Pill>
         </div>
 
-        <div className="settings-toggle-row">
-          <div className="settings-toggle-info">
-            <span className="settings-toggle-label" id={copyOnSelectLabelId}>
-              {t("settings.copy_on_select")}
-            </span>
-            <span className="settings-toggle-desc" id={copyOnSelectDescId}>
-              {t("settings.copy_on_select_hint")}
-            </span>
+        {isMobile ? null : (
+          <div className="settings-toggle-row">
+            <div className="settings-toggle-info">
+              <span className="settings-toggle-label" id={copyOnSelectLabelId}>
+                {t("settings.copy_on_select")}
+              </span>
+              <span className="settings-toggle-desc" id={copyOnSelectDescId}>
+                {t("settings.copy_on_select_hint")}
+              </span>
+            </div>
+            <Switch
+              aria-describedby={copyOnSelectDescId}
+              aria-labelledby={copyOnSelectLabelId}
+              checked={terminalCopyOnSelect}
+              className="settings-toggle"
+              onCheckedChange={(nextValue) => {
+                setTerminalCopyOnSelect(nextValue);
+                void saveSettings({ appearance: { terminalCopyOnSelect: nextValue } });
+              }}
+            />
           </div>
-          <Switch
-            aria-describedby={copyOnSelectDescId}
-            aria-labelledby={copyOnSelectLabelId}
-            checked={terminalCopyOnSelect}
-            className="settings-toggle"
-            onCheckedChange={(nextValue) => {
-              setTerminalCopyOnSelect(nextValue);
-              void saveSettings({ appearance: { terminalCopyOnSelect: nextValue } });
-            }}
-          />
-        </div>
+        )}
       </div>
 
       <div className="settings-group">