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

e2e/settingsPaneLayout.spec.ts

@@ -1,98 +0,0 @@
-import { expect, test } from "./fixtures";
-
-for (const [id, selector] of [
-  ["build.deployment", "[class*='layout'] > [class*='note']"],
-  ["editor.inspections", "[class*='split'] > [class*='details']"],
-  ["languages.servers", "[class*='body'] > [class*='details']"],
-  ["plugins", "[class*='body'] > [class*='details']"],
-  ["tools.agents", "section[aria-label='Codex executable']"],
-]) {
-  test(`${id} keeps its details usable beside the list`, async ({ workbench: page }) => {
-    await page.keyboard.press("Meta+,");
-    const dialog = page.getByRole("dialog", { name: "Settings", exact: true });
-    await expect(dialog).toBeVisible();
-    await page.evaluate(async (id) => {
-      if (id === "tools.agents") {
-        const path = "/src/features/agents/registry.ts";
-        const { tauriCommands } = await import(/* @vite-ignore */ path);
-        tauriCommands.providers = async () => [
-          {
-            id: "codex",
-            name: "Codex",
-            program: "codex",
-            defaultProgram: "codex",
-            args: [],
-            defaultArgs: [],
-            installed: true,
-            commandOverridden: false,
-            env: {},
-          },
-        ];
-      }
-      const path = "/src/features/settings/model/staging.ts";
-      (await import(/* @vite-ignore */ path)).selectPage(id);
-    }, id);
-    const details = dialog.locator(selector).first();
-    await expect(details).toBeAttached();
-    for (const width of [1000, 640, 520]) {
-      await page.setViewportSize({ width, height: 760 });
-      expect((await details.boundingBox())!.width).toBeGreaterThan(180);
-    }
-  });
-}
-
-test("every color scheme page keeps its attribute tree visible", async ({ workbench: page }) => {
-  await page.keyboard.press("Meta+,");
-  await expect(page.getByRole("dialog", { name: "Settings", exact: true })).toBeVisible();
-  await page.setViewportSize({ width: 520, height: 760 });
-  for (const section of [
-    "general",
-    "language-defaults",
-    "debugger",
-    "python",
-    "markdown",
-    "regexp",
-    "java",
-    "kotlin",
-    "php",
-  ]) {
-    await page.evaluate(async (section) => {
-      const path = "/src/features/settings/model/staging.ts";
-      (await import(/* @vite-ignore */ path)).selectPage(`editor.colorScheme.${section}`);
-    }, section);
-    const tree = page.getByRole("tree", { name: "Color scheme attributes" });
-    await expect(tree).toBeAttached();
-    expect((await tree.boundingBox())!.width).toBeGreaterThan(150);
-  }
-});
-
-for (const id of ["editor.fileTemplates", "editor.liveTemplates"]) {
-  test(`${id} keeps its editor reachable beside the template list`, async ({ workbench: page }) => {
-    await page.keyboard.press("Meta+,");
-    await expect(page.getByRole("dialog", { name: "Settings", exact: true })).toBeVisible();
-    await page.evaluate(async (id) => {
-      const path = "/src/features/settings/model/staging.ts";
-      (await import(/* @vite-ignore */ path)).selectPage(id);
-    }, id);
-    await page.setViewportSize({ width: 520, height: 760 });
-    const editor = page.locator("[class*='split'] > [class*='editor']").first();
-    await expect(editor).toBeAttached();
-    expect((await editor.boundingBox())!.width).toBeGreaterThan(200);
-  });
-}
-
-test("Project Structure keeps every details pane usable in a compact window", async ({ workbench: page }) => {
-  await page.keyboard.press("Meta+,");
-  await expect(page.getByRole("dialog", { name: "Settings", exact: true })).toBeVisible();
-  await page.evaluate(async () => {
-    const path = "/src/features/settings/model/staging.ts";
-    (await import(/* @vite-ignore */ path)).selectPage("projectStructure");
-  });
-  await page.setViewportSize({ width: 520, height: 760 });
-  for (const section of ["Modules", "Libraries", "Facets", "Artifacts", "SDKs"]) {
-    await page.getByRole("tab", { name: section, exact: true }).click();
-    const details = page.locator("[class*='splitLayout'] > [class*='detailPane']");
-    await expect(details).toBeAttached();
-    expect((await details.boundingBox())!.width).toBeGreaterThan(200);
-  }
-});

src/features/agents/BuiltinAgents.module.css

@@ -1,6 +1,5 @@
 .split {
   display: flex;
-  min-width: 640px;
   flex: none;
   min-height: 320px;
   border: 1px solid var(--border-strong);

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);
 }

src/features/plugins/PluginsPage.module.css

@@ -43,7 +43,6 @@
 }
 .body {
   display: flex;
-  min-width: 640px;
   flex: 1;
   min-height: 0;
   border-radius: var(--radius-small);

src/features/settings/languages/Sdk.module.css

@@ -212,7 +212,7 @@
 /* Project Structure */
 .structure {
   display: grid;
-  grid-template-columns: minmax(240px, 1fr) 220px;
+  grid-template-columns: minmax(0, 1fr) 220px;
   gap: 10px;
   min-height: 420px;
 }
@@ -418,7 +418,7 @@
 }
 .splitLayout {
   display: grid;
-  grid-template-columns: 268px minmax(300px, 1fr);
+  grid-template-columns: 268px minmax(0, 1fr);
   gap: 12px;
   min-height: 360px;
 }

src/features/settings/pages/Deployment.module.css

@@ -1,6 +1,6 @@
 .layout {
   display: grid;
-  grid-template-columns: 200px minmax(220px, 1fr);
+  grid-template-columns: 200px minmax(0, 1fr);
   gap: 14px;
   align-items: start;
 }

src/features/settings/pages/editor/Editor.module.css

@@ -176,7 +176,7 @@
 }
 .schemeBody {
   display: grid;
-  grid-template-columns: minmax(180px, 1fr) minmax(220px, 260px);
+  grid-template-columns: minmax(0, 1fr) minmax(220px, 260px);
   gap: 14px;
   min-height: 320px;
 }

src/features/settings/pages/editor/Inspections.module.css

@@ -27,7 +27,6 @@
 }
 .split {
   flex: 1;
-  min-width: 520px;
   min-height: 0;
   display: flex;
   gap: 12px;

src/features/templates/settings/Settings.module.css

@@ -14,7 +14,6 @@
 }
 .split {
   flex: 1;
-  min-width: 520px;
   min-height: 320px;
   display: flex;
   border: 1px solid var(--border-strong);

src/features/tools/LanguageServersPage.module.css

@@ -13,7 +13,6 @@
 }
 .body {
   display: flex;
-  min-width: 600px;
   flex: 1;
   min-height: 0;
   border-radius: var(--radius-small);
@@ -170,7 +169,6 @@
 }
 .header {
   display: flex;
-  flex-wrap: wrap;
   align-items: flex-start;
   gap: 12px;
 }