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

e2e/settings.spec.ts

@@ -1,45 +1,5 @@
 import { expect, test } from "./fixtures";
 
-test("Add Python Interpreter keeps environment labels and fields inside their rows", async ({ workbench: page }) => {
-  await page.keyboard.press("Meta+,");
-  const settings = page.getByRole("dialog", { name: "Settings", exact: true });
-  await settings.getByRole("textbox", { name: "Search settings" }).fill("Python Interpreter");
-  await settings
-    .getByRole("tree", { name: "Settings pages" })
-    .getByRole("treeitem", { name: "Python Interpreter", exact: true })
-    .click();
-  await settings.getByRole("button", { name: "Add Interpreter…", exact: true }).click();
-  const dialog = page.getByRole("dialog", { name: "Add Python Interpreter", exact: true });
-  await expect(dialog).toBeVisible();
-
-  for (const width of [1000, 800, 520]) {
-    await page.setViewportSize({ width, height: 760 });
-    await expect(async () => {
-      const labelsFit = await dialog.getByRole("list", { name: "Interpreter type" }).evaluate((list) =>
-        [...list.querySelectorAll("button")].every((button) => {
-          const box = button.getBoundingClientRect();
-          const range = document.createRange();
-          range.selectNodeContents(button);
-          return [...range.getClientRects()].every(
-            (r) => r.top >= box.top - 1 && r.bottom <= box.bottom + 1 && r.right <= box.right + 1,
-          );
-        }),
-      );
-      expect(labelsFit).toBe(true);
-    }).toPass({ timeout: 3000 });
-
-    const location = dialog.getByRole("textbox", { name: "Location:", exact: true });
-    const fieldSpacing = await location.evaluate((input) => ({
-      rowHeight: input.parentElement!.getBoundingClientRect().height,
-      fieldHeight: input.getBoundingClientRect().height,
-    }));
-    expect(fieldSpacing.rowHeight).toBeLessThanOrEqual(fieldSpacing.fieldHeight + 1);
-    const base = await dialog.getByRole("button", { name: "Base interpreter", exact: true }).boundingBox();
-    const loc = await location.boundingBox();
-    expect(base!.y - loc!.y - loc!.height).toBeLessThan(50);
-  }
-});
-
 test("Project Structure keeps edits across repeated shortcuts and applies them", async ({ workbench: page }) => {
   await page.keyboard.press("Meta+;");
   const dialog = page.getByRole("dialog", { name: "Project Structure", exact: true });

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/settings/languages/AddInterpreterDialog.tsx

@@ -226,7 +226,7 @@ export function AddInterpreterDialog({
               <label htmlFor={ids.existing}>
                 <T>Interpreter:</T>
               </label>
-              <div className={styles.pathRow}>
+              <div className={`${styles.pathRow} ${styles.existingPathRow}`}>
                 <Select
                   id={ids.existing}
                   className={styles.pathInput}

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

@@ -152,7 +152,7 @@
 /* Add Python Interpreter dialog */
 .dialogBody {
   display: grid;
-  grid-template-columns: minmax(150px, max-content) minmax(0, 1fr);
+  grid-template-columns: 150px minmax(0, 1fr);
   min-height: 300px;
 }
 .kinds {
@@ -174,7 +174,6 @@
   color: var(--fg);
   font: inherit;
   text-align: left;
-  white-space: nowrap;
 }
 .kindOn {
   background: var(--bg-list-selected);
@@ -188,14 +187,12 @@
 }
 .radios {
   display: flex;
-  flex-wrap: wrap;
-  gap: 6px 16px;
+  gap: 16px;
 }
 .radio {
   display: inline-flex;
   align-items: center;
   gap: 6px;
-  white-space: nowrap;
 }
 .pathRow {
   display: flex;
@@ -204,7 +201,7 @@
   flex: 1;
   min-width: 0;
 }
-.form > .pathRow {
+.existingPathRow {
   flex: none;
 }
 .pathInput {