feat(ext): theme options page + finish popup save UX (#42, #43, #44)

packages/extension-chrome/README.md

@@ -41,9 +41,11 @@ at build time (see "Icons" below).
 2. Click the toolbar icon → "Set up gitmarks" → enter PAT, owner, repo,
    branch.
 3. Optionally enable **"Strip tracking parameters"** to remove `utm_*`,
-   `fbclid`, `gclid`, `msclkid`, and `mc_*` from saved URLs. Default off
-   per the spec's open-question rationale (some sites use `utm_*` for
-   non-tracking purposes).
+   `fbclid`, `gclid`, `msclkid`, and `mc_*` from saved URLs. Links shared
+   from social posts or newsletters carry these; stripping them means the
+   same article saved from two sources collapses to one bookmark, not two.
+   Default off per the spec's open-question rationale (some sites use
+   `utm_*` for non-tracking purposes).
 4. Click **Validate**. You should see either
    "✓ valid PAT, repo exists, bookmarks.json found" or
    "✓ valid PAT, repo exists (bookmarks.json not yet created — will be on

packages/extension-shared/src/lib/save-result-view.ts

@@ -0,0 +1,36 @@
+import type { SaveResult } from "./save-flow.js";
+
+/**
+ * Apply the outcome of a popup save to the button + status line.
+ *
+ * On success: show "✓ saved", relabel the button to "Saved ✓" and mark it
+ * `done` (the CSS for `.done` cancels the disabled progress-cursor), then
+ * auto-dismiss the popup after `closeDelayMs` so it gets out of the way. On
+ * failure: surface the message and re-enable the button to retry.
+ *
+ * Extracted from popup.ts so these transitions are unit-testable without the
+ * popup entry's top-level DOM bootstrap (issue #43). The success branch
+ * previously left the button stuck on "saving…" with a progress cursor.
+ */
+export function applySaveResult(
+  saveBtn: HTMLButtonElement,
+  status: HTMLElement,
+  result: SaveResult,
+  options: { closePopup?: () => void; closeDelayMs?: number } = {},
+): void {
+  const { closePopup = () => window.close(), closeDelayMs = 1200 } = options;
+  if (result.ok) {
+    status.className = "ok";
+    status.textContent = "✓ saved";
+    saveBtn.textContent = "Saved ✓";
+    saveBtn.classList.add("done");
+    // Delay leaves screen-reader users time to hear the status update; a no-op
+    // when opened as a plain page (e2e) where window.close() does nothing.
+    setTimeout(closePopup, closeDelayMs);
+  } else {
+    status.className = "err";
+    status.textContent = result.message;
+    saveBtn.disabled = false;
+    saveBtn.textContent = "Try again";
+  }
+}

packages/extension-shared/src/options.html

@@ -4,24 +4,51 @@
     <meta charset="utf-8" />
     <title>gitmarks — settings</title>
     <style>
+      /* Palette mirrors @gitmarks/web (tailwind.config.ts):
+         ink #0a0a0f, mist #16161e, fog #23232e, cyan #22d3ee, magenta #e879f9. */
+      :root { color-scheme: dark; }
       body {
-        font-family: system-ui, sans-serif;
+        font-family: ui-monospace, "JetBrains Mono", SFMono-Regular, Menlo, monospace;
         max-width: 480px;
         margin: 2rem auto;
         padding: 0 1rem;
-        color: #1a1a1a;
-        background: #fafafa;
+        color: #e5e7eb;
+        background: #0a0a0f;
       }
-      h1 { font-size: 1.5rem; margin-bottom: 1.5rem; }
+      h1 { font-size: 1.5rem; margin-bottom: 1.5rem; color: #e879f9; letter-spacing: 0.02em; }
       label { display: block; margin-bottom: 1rem; }
-      label > span { display: block; font-size: 0.85rem; margin-bottom: 0.25rem; }
-      input { width: 100%; padding: 0.5rem; font: inherit; box-sizing: border-box; border: 1px solid #ccc; border-radius: 4px; }
+      label > span { display: block; font-size: 0.85rem; margin-bottom: 0.25rem; color: #cbd5e1; }
+      input {
+        width: 100%;
+        padding: 0.5rem;
+        font: inherit;
+        box-sizing: border-box;
+        border: 1px solid #23232e;
+        border-radius: 6px;
+        background: #16161e;
+        color: #e5e7eb;
+      }
+      input:focus { outline: none; border-color: #22d3ee; }
+      .checkbox { display: flex; align-items: flex-start; gap: 0.5rem; }
+      .checkbox input { width: auto; margin-top: 0.2rem; accent-color: #22d3ee; }
+      .hint { font-size: 0.8rem; color: #9ca3af; margin: 0.35rem 0 0 1.6rem; line-height: 1.45; }
       .row { display: flex; gap: 0.5rem; margin-top: 1.5rem; }
-      button { padding: 0.5rem 1rem; font: inherit; border: 1px solid #444; background: #222; color: white; border-radius: 4px; cursor: pointer; }
-      button.secondary { background: white; color: #222; }
+      button {
+        padding: 0.5rem 1rem;
+        font: inherit;
+        border: 1px solid #22d3ee;
+        background: #22d3ee;
+        color: #0a0a0f;
+        border-radius: 6px;
+        cursor: pointer;
+        transition: background 0.12s, color 0.12s;
+      }
+      button:hover { background: #67e8f9; border-color: #67e8f9; }
+      button.secondary { background: transparent; color: #22d3ee; }
+      button.secondary:hover { background: #22d3ee; color: #0a0a0f; }
       #status { margin-top: 1rem; font-size: 0.9rem; min-height: 1.2em; }
-      #status.ok { color: #096; }
-      #status.err { color: #c00; }
+      #status.ok { color: #22d3ee; }
+      #status.err { color: #e879f9; }
     </style>
   </head>
   <body>
@@ -51,6 +78,11 @@ <h1>gitmarks settings</h1>
       <input id="stripTrackingParams" type="checkbox" />
       Strip tracking parameters (utm_*, fbclid, gclid, msclkid, mc_*) at save time
     </label>
+    <p class="hint">
+      Links shared from social posts or newsletters often carry tracking
+      parameters that identify who shared them. Stripping these means the same
+      article saved from two sources becomes one bookmark, not two.
+    </p>
 
     <div class="row">
       <button id="validate" class="secondary">Validate</button>

packages/extension-shared/src/popup.html

@@ -35,6 +35,8 @@
       }
       button:hover:not(:disabled) { background: #22d3ee; color: #0a0a0f; }
       button:disabled { opacity: 0.5; cursor: progress; }
+      /* Success state: stays disabled (no re-save) but reads as done, not busy. */
+      button.done { opacity: 1; cursor: default; background: transparent; color: #22d3ee; }
       #status { margin-top: 0.75rem; font-size: 0.85rem; min-height: 1.2em; }
       #status.ok { color: #22d3ee; }
       #status.err { color: #e879f9; }

packages/extension-shared/src/popup.ts

@@ -3,6 +3,7 @@ import { GitHubClient } from "@gitmarks/core";
 import { loadSettings, SettingsCorruptError } from "./lib/settings.js";
 import { getMachineId } from "./lib/machine-id.js";
 import { saveBookmark, type SaveResult } from "./lib/save-flow.js";
+import { applySaveResult } from "./lib/save-result-view.js";
 import type { LastErrorRecord } from "./lib/background-core.js";
 
 const root = document.getElementById("root");
@@ -104,18 +105,7 @@ async function render(): Promise<void> {
         message: err instanceof Error ? err.message : String(err),
       };
     }
-    if (result.ok) {
-      status.className = "ok";
-      status.textContent = "✓ saved";
-      // Auto-dismiss the popup shortly after a successful save so the user
-      // doesn't have to click away. No-op when opened as a plain page (e2e).
-      setTimeout(() => window.close(), 1200);
-    } else {
-      status.className = "err";
-      status.textContent = result.message;
-      saveBtn.disabled = false;
-      saveBtn.textContent = "Try again";
-    }
+    applySaveResult(saveBtn, status, result);
   });
 }
 

packages/extension-shared/test/save-result-view.test.ts

@@ -0,0 +1,59 @@
+import { describe, it, expect, vi, beforeEach, afterEach } from "vitest";
+import type { Bookmark } from "@gitmarks/core";
+import { applySaveResult } from "../src/lib/save-result-view.js";
+
+// applySaveResult ignores the bookmark payload; a cast keeps the fixture terse.
+const okResult = { ok: true as const, bookmark: {} as Bookmark };
+
+function makeDom(): { saveBtn: HTMLButtonElement; status: HTMLElement } {
+  const saveBtn = document.createElement("button");
+  saveBtn.disabled = true;
+  saveBtn.textContent = "saving…";
+  const status = document.createElement("p");
+  return { saveBtn, status };
+}
+
+describe("applySaveResult", () => {
+  beforeEach(() => vi.useFakeTimers());
+  afterEach(() => vi.useRealTimers());
+
+  it("on success: shows saved, relabels button, marks it done, and auto-closes", () => {
+    const { saveBtn, status } = makeDom();
+    const closePopup = vi.fn();
+
+    applySaveResult(saveBtn, status, okResult, { closePopup, closeDelayMs: 1200 });
+
+    expect(status.className).toBe("ok");
+    expect(status.textContent).toBe("✓ saved");
+    expect(saveBtn.textContent).toBe("Saved ✓");
+    // No longer stuck on "saving…"; the `done` class cancels the progress cursor.
+    expect(saveBtn.classList.contains("done")).toBe(true);
+
+    // Popup is not dismissed before the delay elapses...
+    expect(closePopup).not.toHaveBeenCalled();
+    vi.advanceTimersByTime(1200);
+    // ...and exactly once after.
+    expect(closePopup).toHaveBeenCalledTimes(1);
+  });
+
+  it("on failure: surfaces the message, re-enables the button, never closes", () => {
+    const { saveBtn, status } = makeDom();
+    const closePopup = vi.fn();
+
+    applySaveResult(
+      saveBtn,
+      status,
+      { ok: false, kind: "unknown", message: "boom" },
+      { closePopup },
+    );
+
+    expect(status.className).toBe("err");
+    expect(status.textContent).toBe("boom");
+    expect(saveBtn.disabled).toBe(false);
+    expect(saveBtn.textContent).toBe("Try again");
+    expect(saveBtn.classList.contains("done")).toBe(false);
+
+    vi.advanceTimersByTime(10_000);
+    expect(closePopup).not.toHaveBeenCalled();
+  });
+});