fix(github): expose review settings in the native settings window

e2e/databaseDialogLayout.spec.ts

@@ -1,34 +0,0 @@
-import { expect, test } from "./fixtures";
-
-for (const kind of ["password", "sshSecret", "params"] as const) {
-  test(`database ${kind} dialog keeps its fields inside the window`, async ({ workbench: page }) => {
-    await page.evaluate(async (kind) => {
-      const path = "/src/features/database/store.ts";
-      const { openDialog } = await import(/* @vite-ignore */ path);
-      const source = {
-        id: "layout-test",
-        name: "Layout test",
-        driver: "postgres",
-        user: "account_".repeat(30),
-        ssh: { enabled: true, host: "bastion.example.invalid", auth: "password" },
-      };
-      if (kind === "password") openDialog({ kind, source, resolve: () => {} });
-      else if (kind === "sshSecret")
-        openDialog({ kind, source, prompt: "SSH password required for " + "host_".repeat(30), resolve: () => {} });
-      else openDialog({ kind, names: ["parameter_".repeat(25)], resolve: () => {} });
-    }, kind);
-    const dialog = page.getByRole("dialog");
-    await expect(dialog).toBeVisible();
-    for (const width of [1000, 640, 500]) {
-      await page.setViewportSize({ width, height: 760 });
-      const fits = await dialog.evaluate((dialog) => {
-        const box = dialog.getBoundingClientRect();
-        return [...dialog.querySelectorAll("input, [class*='form_']")].every((el) => {
-          const r = el.getBoundingClientRect();
-          return r.left >= box.left && r.right <= box.right && el.scrollWidth <= el.clientWidth + 1;
-        });
-      });
-      expect(fits).toBe(true);
-    }
-  });
-}

src/features/database/Database.module.css

@@ -350,15 +350,11 @@
 /* Dialogs. */
 .form {
   display: grid;
-  grid-template-columns: fit-content(40%) minmax(0, 1fr);
+  grid-template-columns: max-content 1fr;
   align-items: center;
   gap: 8px 12px;
-  min-width: 0;
+  min-width: 440px;
   padding: 14px;
-  overflow-wrap: anywhere;
-}
-.form > * {
-  min-width: 0;
 }
 .formLabel {
   color: var(--fg-secondary);

src/features/github/diff/index.ts

@@ -3,14 +3,11 @@
  * (opened by `openPullRequestFileDiff` / the details panel's commit diffs, context
  * `PULL_REQUEST_DIFF`) get their review threads as inlays, the gutter "+" with single- and
  * multi-line comments, Start Review / Add Single Comment, the review toolbar and the viewed
- * toggle, through the vcs diff viewer provider point. Also registers the editor actions and
- * Settings › Version Control › GitHub. The editor actions also serve review mode's editors.
+ * toggle, through the vcs diff viewer provider point. The editor actions also serve review mode's editors.
  */
 import { registerDiffViewerProvider } from "@/features/vcs/diffViewerProviders";
 import { registerActions } from "@/platform/actions";
 import { getContext } from "@/platform/context";
-import { lazyComponent } from "@/ui";
-import { registerSettingsPage } from "@/workbench/contributions";
 import { useGhRepository } from "../data/repository";
 import { activeReviewEditor } from "../reviewMode/editor";
 import { clearNewComments } from "./newComments";
@@ -82,13 +79,6 @@ export function activateReviewDiff(): () => void {
         run: () => reviewTarget()?.refresh(),
       },
     ]),
-    registerSettingsPage({
-      id: "vcs.github",
-      title: "GitHub",
-      order: 9,
-      keywords: ["pull request", "viewed", "review"],
-      component: lazyComponent(() => import("./GithubSettingsPage").then((m) => m.GithubSettingsPage)),
-    }),
     useGhRepository.subscribe((s, prev) => {
       const a = s.connection?.coordinates;
       const b = prev.connection?.coordinates;

src/features/github/diff/settings.ts

@@ -46,6 +46,10 @@ function load(): ReviewDiffSettings {
 
 export const useReviewDiffSettings = create<ReviewDiffSettings>(load);
 
+globalThis.addEventListener?.("storage", (event: StorageEvent) => {
+  if (event.key === KEY || event.key === null) useReviewDiffSettings.setState(load(), true);
+});
+
 export function setReviewDiffSettings(patch: Partial<ReviewDiffSettings>) {
   useReviewDiffSettings.setState(patch);
   try {

src/features/github/index.ts

@@ -22,6 +22,9 @@ import {
 import { useGithubShell } from "./shell";
 import { githubHandler, handlerContextKey } from "./toolwindow/handlers";
 import { GithubDialogHost } from "./ui/dialogs";
+import { activateSettingsPages } from "./settingsPages";
+
+export { activateSettingsPages } from "./settingsPages";
 
 const WINDOW = "pullRequests";
 const TIMELINE_KIND = "github.timeline";
@@ -255,6 +258,7 @@ export function activate(): () => void {
   active = true;
   setContext("githubPrTab", "list");
   const disposers = [
+    activateSettingsPages(),
     registerCloneSource({
       id: "github",
       label: "GitHub",

src/features/github/settingsPages.test.tsx

@@ -0,0 +1,52 @@
+import { beforeEach, describe, expect, it } from "vitest";
+import { act, fireEvent, render, screen } from "@testing-library/react";
+import { useSettingsPages } from "@/workbench/contributions";
+import { activateSettingsPages } from "./settingsPages";
+import { GithubSettingsPage } from "./diff/GithubSettingsPage";
+import { setReviewDiffSettings, useReviewDiffSettings } from "./diff/settings";
+
+const KEY = "openidea.github.reviewDiff.v1";
+
+beforeEach(() => {
+  localStorage.removeItem(KEY);
+  window.dispatchEvent(new StorageEvent("storage", { key: KEY }));
+});
+
+describe("GitHub settings pages", () => {
+  it("registers independently of the pull request runtime", () => {
+    const dispose = activateSettingsPages();
+    expect(useSettingsPages.getState().pages.get("vcs.github")).toMatchObject({ title: "GitHub" });
+    expect(useSettingsPages.getState().pages.get("vcs.github")?.component).toBeDefined();
+    dispose();
+    expect(useSettingsPages.getState().pages.has("vcs.github")).toBe(false);
+  });
+
+  it("updates an open page when another window saves or clears review settings", () => {
+    render(<GithubSettingsPage />);
+    const checkbox = screen.getByRole("checkbox", { name: "Automatically mark opened files as viewed" });
+    expect(checkbox).not.toBeChecked();
+    act(() => {
+      localStorage.setItem(KEY, JSON.stringify({ autoMarkViewed: true, threadsView: "all" }));
+      window.dispatchEvent(new StorageEvent("storage", { key: KEY }));
+    });
+    expect(checkbox).toBeChecked();
+    expect(useReviewDiffSettings.getState().threadsView).toBe("all");
+    act(() => {
+      localStorage.clear();
+      window.dispatchEvent(new StorageEvent("storage", { key: null }));
+    });
+    expect(checkbox).not.toBeChecked();
+    fireEvent.click(checkbox);
+    expect(JSON.parse(localStorage.getItem(KEY)!)).toMatchObject({ autoMarkViewed: true });
+  });
+
+  it("ignores unrelated storage changes and recovers from invalid saved settings", () => {
+    setReviewDiffSettings({ autoMarkViewed: true });
+    localStorage.removeItem(KEY);
+    window.dispatchEvent(new StorageEvent("storage", { key: "other" }));
+    expect(useReviewDiffSettings.getState().autoMarkViewed).toBe(true);
+    localStorage.setItem(KEY, "invalid JSON");
+    window.dispatchEvent(new StorageEvent("storage", { key: KEY }));
+    expect(useReviewDiffSettings.getState().autoMarkViewed).toBe(false);
+  });
+});

src/features/github/settingsPages.ts

@@ -0,0 +1,12 @@
+import { lazyComponent } from "@/ui";
+import { registerSettingsPage } from "@/workbench/contributions";
+
+export function activateSettingsPages(): () => void {
+  return registerSettingsPage({
+    id: "vcs.github",
+    title: "GitHub",
+    order: 9,
+    keywords: ["pull request", "viewed", "review"],
+    component: lazyComponent(() => import("./diff/GithubSettingsPage").then((m) => m.GithubSettingsPage)),
+  });
+}

src/features/index.ts

@@ -43,7 +43,7 @@ import { activate as notebook } from "./notebook";
 import { activate as web } from "./web";
 import { activate as analysis } from "./analysis";
 import { activate as services } from "./services";
-import { activate as github } from "./github";
+import { activate as github, activateSettingsPages as githubPages } from "./github";
 import { activate as httpClient } from "./http-client";
 import { activate as database } from "./database";
 import { activate as graphql } from "./graphql";
@@ -166,6 +166,7 @@ export function activateSettingsWindowFeatures(parent: string): () => void {
     ["agents", agentsPages],
     ["vim", vimPages],
     ["vcs", vcsPages],
+    ["github", githubPages],
   ];
   return activateAll("settings pages", pages);
 }