From 5cc71c3050d5d040c7106d8242dacf5ce2377777 Mon Sep 17 00:00:00 2001
From: Bryan Van Deusen <bryan.vandeusen@fabledsword.com>
Date: Fri, 16 Oct 2026 13:41:03 -0400
Subject: [PATCH] Settings: tabs instead of one long page, and a review pass
 (#5916)

Settings was every registry group, then invites, accounts, groups and
activity, in one scroll. It is now six tabs held in ?tab=:
- General: General, Notes, Attachments, Links.
- People: Access, Invites, Accounts, Groups.
- Email, with a line saying whether email is on and what it still needs.
- Security.
- Activity.
A registry group that no tab names lands on General, so it can't go missing.

Save saves only the current tab. Edits waiting on other tabs keep their
values and mark their tab with a dot. Leaving the page with unsaved edits
asks first.

Review fixes:
- On a phone, a field stacks under its label instead of overflowing beside
  it.
- Descriptions are easier to read.
- The SMTP encryption choice shows readable names (choice_labels).
- The test-email button uses .btn-sm and is disabled while email is off.
- The accounts list is headed Accounts, not People, now that People is a
  tab.
- The docs point at the new paths.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
---
 docs/public-hosting.md                  |   6 +-
 frontend/src/adapters/repo.ts           |   2 +
 frontend/src/components/AccountList.vue |   2 +-
 frontend/src/views/SettingsView.vue     | 299 +++++++++++++++++-------
 src/inkwell/settings.py                 |   5 +
 src/inkwell/storage.py                  |   2 +-
 tests/test_settings.py                  |   8 +
 7 files changed, 241 insertions(+), 83 deletions(-)

diff --git a/docs/public-hosting.md b/docs/public-hosting.md
index f5b6716..11ca407 100644
--- a/docs/public-hosting.md
+++ b/docs/public-hosting.md
@@ -23,10 +23,10 @@ matters.
 **Instances that predate this still need one manual flip.** The close fires when the
 first account is created, so a server whose admin already existed keeps whatever
 `allow_registration` was set to — which was **on** by default. Check **Settings →
-Access → Allow new registrations** before exposing an instance you have been running
+People → Allow new registrations** before exposing an instance you have been running
 on a LAN.
 
-To let someone else in, send them an invite: **Settings → Invites** makes a link that
+To let someone else in, send them an invite: **Settings → People → Invites** makes a link that
 works once, expires (a week by default), and can be pinned to one email address. It
 lets that one person register while registration stays closed.
 
@@ -146,7 +146,7 @@ address can be sent.
 ## Storage per account
 
 Each account may store up to `storage_quota_gb` of attachments (Settings →
-Attachments, 5 GB by default; 0 means no limit). Admins have no limit. Trashed notes
+General → Attachments, 5 GB by default; 0 means no limit). Admins have no limit. Trashed notes
 count until the trash is emptied, because their files are still on disk. An upload
 past the limit is refused with 507 Insufficient Storage, and a phone or desktop
 retries that file on each sync, so it goes through once there is room. The Account
diff --git a/frontend/src/adapters/repo.ts b/frontend/src/adapters/repo.ts
index 346b0fa..f402716 100644
--- a/frontend/src/adapters/repo.ts
+++ b/frontend/src/adapters/repo.ts
@@ -189,6 +189,8 @@ export interface ServerSetting {
   maximum: number | null;
   /** Strings: the allowed values, drawn as a choice. */
   choices: string[] | null;
+  /** What each choice is called, in the same order; null to show the values. */
+  choice_labels: string[] | null;
   /** A credential (the SMTP password). Its value never comes back; `is_set` says
    *  whether one is saved, and leaving the field empty keeps it. */
   secret: boolean;
diff --git a/frontend/src/components/AccountList.vue b/frontend/src/components/AccountList.vue
index f1f6abb..3c29324 100644
--- a/frontend/src/components/AccountList.vue
+++ b/frontend/src/components/AccountList.vue
@@ -43,7 +43,7 @@ onMounted(() => {
 
 <template>
   <section class="flex flex-col gap-5">
-    <h2 class="section-label">People</h2>
+    <h2 class="section-label">Accounts</h2>
 
     <OneTimeSecret
       v-if="fresh"
diff --git a/frontend/src/views/SettingsView.vue b/frontend/src/views/SettingsView.vue
index cee6c63..e41a2b8 100644
--- a/frontend/src/views/SettingsView.vue
+++ b/frontend/src/views/SettingsView.vue
@@ -1,5 +1,6 @@
 <script setup lang="ts">
-import { computed, onMounted, ref, watch } from "vue";
+import { computed, onBeforeUnmount, onMounted, ref, watch } from "vue";
+import { onBeforeRouteLeave, useRoute, useRouter } from "vue-router";
 import { repo } from "../adapters";
 import type { ServerSetting, SettingValue } from "../adapters/repo";
 import { useConfigStore } from "../stores/config";
@@ -15,7 +16,44 @@ import { useUiStore } from "../stores/ui";
 
 const config = useConfigStore();
 const ui = useUiStore();
+const route = useRoute();
+const router = useRouter();
 
+// ---- tabs ----
+//
+// One page per subject, rather than every setting the server has in one scroll. The
+// tab is in the URL (`?tab=email`), so a link can open the right one and a reload
+// keeps it.
+interface Tab {
+  id: string;
+  label: string;
+  /** The registry groups whose fields this tab holds. */
+  groups: string[];
+}
+const TABS: Tab[] = [
+  { id: "general", label: "General", groups: ["General", "Notes", "Attachments", "Links"] },
+  { id: "people", label: "People", groups: ["Access"] },
+  { id: "email", label: "Email", groups: ["Email"] },
+  { id: "security", label: "Security", groups: ["Security"] },
+  { id: "activity", label: "Activity", groups: [] },
+];
+const DEFAULT_TAB = "general";
+
+/** The tab a registry group is shown on. A group no tab names lands on General, so a
+ *  setting added to the registry always has somewhere to be changed. */
+function tabOf(group: string): string {
+  return TABS.find((t) => t.groups.includes(group))?.id ?? DEFAULT_TAB;
+}
+
+const tab = computed(() => {
+  const q = route.query.tab;
+  return TABS.some((t) => t.id === q) ? String(q) : DEFAULT_TAB;
+});
+function openTab(id: string): void {
+  void router.replace({ query: { ...route.query, tab: id === DEFAULT_TAB ? undefined : id } });
+}
+
+// ---- the registry's fields ----
 const items = ref<ServerSetting[]>([]);
 const original = ref<Record<string, SettingValue>>({});
 const loading = ref(true);
@@ -24,15 +62,25 @@ const saving = ref(false);
 const error = ref("");
 const saved = ref(false);
 
-// Unsaved-changes tracking: compare current values to the last loaded/saved snapshot.
-const dirty = computed(() => items.value.some((it) => it.value !== original.value[it.key]));
-watch(dirty, (d) => {
+const isDirty = (it: ServerSetting) => it.value !== original.value[it.key];
+const tabItems = computed(() => items.value.filter((it) => tabOf(it.group) === tab.value));
+const tabDirty = computed(() => tabItems.value.some(isDirty));
+/** Tabs holding edits not yet saved, marked in the tab bar so switching away from
+ *  one doesn't hide that it still needs saving. */
+const dirtyTabs = computed(() => new Set(items.value.filter(isDirty).map((it) => tabOf(it.group))));
+
+watch(tabDirty, (d) => {
   if (d) saved.value = false;
 });
+watch(tab, () => {
+  saved.value = false;
+  error.value = "";
+});
 
+// The current tab's fields, under their group's heading, in registry order.
 const groups = computed(() => {
   const map = new Map<string, ServerSetting[]>();
-  for (const it of items.value) {
+  for (const it of tabItems.value) {
     const arr = map.get(it.group) ?? [];
     arr.push(it);
     map.set(it.group, arr);
@@ -57,15 +105,19 @@ async function load() {
   }
 }
 
+/** Saves THIS tab's changes. Edits waiting on other tabs are kept as they are,
+ *  still unsaved, rather than replaced by the server's copy. */
 async function save() {
   error.value = "";
   saved.value = false;
   saving.value = true;
   const payload: Record<string, SettingValue> = {};
-  for (const it of items.value) payload[it.key] = it.value;
+  for (const it of tabItems.value) if (isDirty(it)) payload[it.key] = it.value;
+  const pending = new Map(items.value.filter((it) => isDirty(it) && !(it.key in payload)).map((it) => [it.key, it.value]));
   try {
-    items.value = await repo.settings.save(payload);
-    snapshot(items.value);
+    const fresh = await repo.settings.save(payload);
+    snapshot(fresh);
+    items.value = fresh.map((it) => (pending.has(it.key) ? { ...it, value: pending.get(it.key) as SettingValue } : it));
     saved.value = true;
     await config.reload(); // header/site name reflects changes immediately
   } catch (e) {
@@ -75,6 +127,34 @@ async function save() {
   }
 }
 
+/** The label a choice is shown under: the registry's, or the stored value. */
+function choiceLabel(it: ServerSetting, i: number): string {
+  return it.choice_labels?.[i] ?? it.choices?.[i] ?? "";
+}
+
+// ---- leaving with unsaved edits ----
+const anyDirty = computed(() => dirtyTabs.value.size > 0);
+onBeforeRouteLeave(() => !anyDirty.value || window.confirm("Leave Settings without saving your changes?"));
+function onBeforeUnload(e: BeforeUnloadEvent) {
+  if (anyDirty.value) e.preventDefault();
+}
+onMounted(() => window.addEventListener("beforeunload", onBeforeUnload));
+onBeforeUnmount(() => window.removeEventListener("beforeunload", onBeforeUnload));
+
+// ---- email ----
+//
+// Whether email is on, from the SAVED values: it is what a reset email would use, and
+// the server's own test (`mail_configured`) is the same three things.
+const savedValue = (key: string) => String(original.value[key] ?? "").trim();
+const mailMissing = computed(() => {
+  const missing: string[] = [];
+  if (!savedValue("smtp_host")) missing.push("an SMTP server");
+  if (!savedValue("smtp_from") && !savedValue("smtp_username")) missing.push("a from address");
+  if (!savedValue("public_url")) missing.push("the public address (on General)");
+  return missing;
+});
+const mailSender = computed(() => savedValue("smtp_from") || savedValue("smtp_username"));
+
 // Sends with the SAVED settings, which is what a reset email will use, so it waits
 // for unsaved changes to be saved first.
 const testing = ref(false);
@@ -97,89 +177,152 @@ onMounted(load);
   <div class="page-shell">
     <PageHeader title="Settings" />
 
+    <!-- Scrolls sideways inside itself on a narrow screen rather than wrapping: a
+         wrapped tab bar reads as two rows of unrelated buttons. -->
+    <div
+      role="tablist"
+      aria-label="Settings sections"
+      class="-mx-4 mb-8 flex gap-1 overflow-x-auto border-b border-neutral-200 px-4 dark:border-neutral-800"
+    >
+      <button
+        v-for="t in TABS"
+        :key="t.id"
+        type="button"
+        role="tab"
+        :aria-selected="tab === t.id"
+        class="-mb-px flex shrink-0 items-center gap-1.5 border-b-2 px-3 py-2 text-sm font-medium transition focus:outline-none focus-visible:ring-2 focus-visible:ring-brand"
+        :class="
+          tab === t.id
+            ? 'border-brand text-neutral-900 dark:text-neutral-100'
+            : 'border-transparent text-neutral-500 hover:text-neutral-800 dark:text-neutral-400 dark:hover:text-neutral-200'
+        "
+        @click="openTab(t.id)"
+      >
+        {{ t.label }}
+        <span
+          v-if="dirtyTabs.has(t.id)"
+          class="h-1.5 w-1.5 rounded-full bg-brand"
+          title="Unsaved changes"
+          aria-label="unsaved changes"
+        ></span>
+      </button>
+    </div>
+
     <AsyncState :loading="loading" :error="loadError" error-title="Couldn't load settings" @retry="load">
-      <form class="flex flex-col gap-8" @submit.prevent="save">
-        <section v-for="group in groups" :key="group.name" class="flex flex-col gap-5">
-          <h2 class="section-label">{{ group.name }}</h2>
+      <div role="tabpanel" class="flex flex-col gap-10">
+        <!-- Email leads with whether it works, before the fields that decide it. -->
+        <p
+          v-if="tab === 'email'"
+          class="flex items-start gap-2 rounded-lg border px-3 py-2 text-sm"
+          :class="
+            mailMissing.length
+              ? 'border-amber-300 bg-amber-50 text-amber-900 dark:border-amber-800 dark:bg-amber-950/40 dark:text-amber-200'
+              : 'border-green-300 bg-green-50 text-green-900 dark:border-green-900 dark:bg-green-950/40 dark:text-green-200'
+          "
+        >
+          <template v-if="mailMissing.length">
+            Email is off. Password reset and invite emails need {{ mailMissing.join(", ") }}.
+          </template>
+          <template v-else>Email is on, sending as {{ mailSender }}.</template>
+        </p>
 
-          <div v-for="it in group.entries" :key="it.key" class="flex flex-col gap-1">
-            <div class="flex items-center justify-between gap-6">
-              <label :for="it.key" class="text-sm font-medium text-neutral-800 dark:text-neutral-200">{{
-                it.label
-              }}</label>
+        <form v-if="groups.length" class="flex flex-col gap-8" @submit.prevent="save">
+          <section v-for="group in groups" :key="group.name" class="flex flex-col gap-5">
+            <h2 class="section-label">{{ group.name }}</h2>
 
-              <input
-                v-if="it.type === 'bool'"
-                :id="it.key"
-                type="checkbox"
-                class="h-4 w-4 accent-brand focus:outline-none focus-visible:ring-2 focus-visible:ring-brand"
-                :checked="Boolean(it.value)"
-                @change="it.value = ($event.target as HTMLInputElement).checked"
-              />
-              <!-- min/max come from the registry. The server rejects out-of-range
-                   values regardless — this is so the browser says so first, rather than
-                   letting someone type a hop count that would disable a protection and
-                   only learn about it from an error banner. -->
-              <input
-                v-else-if="it.type === 'int'"
-                :id="it.key"
-                type="number"
-                :min="it.minimum ?? undefined"
-                :max="it.maximum ?? undefined"
-                class="w-28 field"
-                :value="Number(it.value)"
-                @input="it.value = Number(($event.target as HTMLInputElement).value)"
-              />
-              <select
-                v-else-if="it.choices"
-                :id="it.key"
-                class="w-56 field"
-                :value="String(it.value)"
-                @change="it.value = ($event.target as HTMLSelectElement).value"
-              >
-                <option v-for="c in it.choices" :key="c" :value="c">{{ c }}</option>
-              </select>
-              <input
-                v-else
-                :id="it.key"
-                :type="it.secret ? 'password' : 'text'"
-                :autocomplete="it.secret ? 'new-password' : undefined"
-                :placeholder="it.secret && it.is_set ? 'Saved — type to replace' : undefined"
-                class="w-56 field"
-                :value="String(it.value)"
-                @input="it.value = ($event.target as HTMLInputElement).value"
-              />
+            <!-- Stacked on a phone, label beside field from sm up: a fixed-width field
+                 next to a long label does not fit 360px. -->
+            <div
+              v-for="it in group.entries"
+              :key="it.key"
+              class="flex flex-col gap-2 sm:flex-row sm:items-start sm:justify-between sm:gap-6"
+            >
+              <div class="min-w-0">
+                <label :for="it.key" class="text-sm font-medium text-neutral-800 dark:text-neutral-200">{{
+                  it.label
+                }}</label>
+                <p class="mt-0.5 max-w-md text-xs text-neutral-500 dark:text-neutral-400">{{ it.description }}</p>
+              </div>
+
+              <div class="shrink-0">
+                <input
+                  v-if="it.type === 'bool'"
+                  :id="it.key"
+                  type="checkbox"
+                  class="mt-1 h-4 w-4 accent-brand focus:outline-none focus-visible:ring-2 focus-visible:ring-brand"
+                  :checked="Boolean(it.value)"
+                  @change="it.value = ($event.target as HTMLInputElement).checked"
+                />
+                <!-- min/max come from the registry. The server rejects out-of-range
+                     values regardless — this is so the browser says so first, rather than
+                     letting someone type a hop count that would disable a protection and
+                     only learn about it from an error banner. -->
+                <input
+                  v-else-if="it.type === 'int'"
+                  :id="it.key"
+                  type="number"
+                  :min="it.minimum ?? undefined"
+                  :max="it.maximum ?? undefined"
+                  class="field w-28"
+                  :value="Number(it.value)"
+                  @input="it.value = Number(($event.target as HTMLInputElement).value)"
+                />
+                <select
+                  v-else-if="it.choices"
+                  :id="it.key"
+                  class="field w-full sm:w-64"
+                  :value="String(it.value)"
+                  @change="it.value = ($event.target as HTMLSelectElement).value"
+                >
+                  <option v-for="(c, i) in it.choices" :key="c" :value="c">{{ choiceLabel(it, i) }}</option>
+                </select>
+                <input
+                  v-else
+                  :id="it.key"
+                  :type="it.secret ? 'password' : 'text'"
+                  :autocomplete="it.secret ? 'new-password' : undefined"
+                  :placeholder="it.secret && it.is_set ? 'Saved — type to replace' : undefined"
+                  class="field w-full sm:w-64"
+                  :value="String(it.value)"
+                  @input="it.value = ($event.target as HTMLInputElement).value"
+                />
+              </div>
             </div>
-            <p class="max-w-md text-xs text-neutral-400">{{ it.description }}</p>
-          </div>
+          </section>
 
-          <div v-if="group.name === 'Email'" class="flex items-center gap-3">
+          <div class="flex flex-wrap items-center gap-3">
+            <BaseButton type="submit" :loading="saving" :disabled="!tabDirty">Save changes</BaseButton>
+            <span v-if="saved && !tabDirty" class="text-sm text-green-600 dark:text-green-400">Saved.</span>
+            <span v-else-if="!tabDirty && !error" class="text-sm text-neutral-400">No unsaved changes</span>
+            <span v-if="error" class="form-error">{{ error }}</span>
+          </div>
+        </form>
+
+        <section v-if="tab === 'email'" class="flex flex-col gap-3">
+          <h2 class="section-label">Check it</h2>
+          <div class="flex flex-wrap items-center gap-3">
             <button
               type="button"
-              class="rounded-md border border-neutral-300 px-3 py-1.5 text-sm hover:bg-neutral-100 focus:outline-none focus-visible:ring-2 focus-visible:ring-brand disabled:opacity-50 dark:border-neutral-700 dark:hover:bg-neutral-800"
-              :disabled="testing || dirty"
+              class="btn-sm"
+              :disabled="testing || tabDirty || mailMissing.length > 0"
               @click="sendTestEmail"
             >
-              Send test email
+              Send a test email to me
             </button>
-            <span v-if="dirty" class="text-xs text-neutral-400">Save first</span>
+            <span v-if="tabDirty" class="text-xs text-neutral-400">Save your changes first.</span>
           </div>
         </section>
 
-        <div class="flex items-center gap-3">
-          <BaseButton type="submit" :loading="saving" :disabled="!dirty">Save changes</BaseButton>
-          <span v-if="saved && !dirty" class="text-sm text-green-600 dark:text-green-400">Saved.</span>
-          <span v-else-if="!dirty && !error" class="text-sm text-neutral-400">No unsaved changes</span>
-          <span v-if="error" class="form-error">{{ error }}</span>
-        </div>
-      </form>
-    </AsyncState>
+        <!-- Each invite, account and group action saves on its own; the form's Save
+             button has nothing to do with them. -->
+        <template v-if="tab === 'people'">
+          <InviteList />
+          <AccountList />
+          <GroupList />
+        </template>
 
-    <!-- Outside the settings form: each invite, account and group action saves on its own,
-         and the form's Save button has nothing to do with them. -->
-    <InviteList v-if="items.length" class="mt-10" />
-    <AccountList v-if="items.length" class="mt-10" />
-    <GroupList v-if="items.length" class="mt-10" />
-    <ActivityList v-if="items.length" class="mt-10" />
+        <ActivityList v-if="tab === 'activity'" />
+      </div>
+    </AsyncState>
   </div>
 </template>
diff --git a/src/inkwell/settings.py b/src/inkwell/settings.py
index 37c26a1..6982b03 100644
--- a/src/inkwell/settings.py
+++ b/src/inkwell/settings.py
@@ -38,6 +38,9 @@ class SettingDef:
     maximum: int | None = None
     # Strings only: the values allowed, shown as a choice rather than a text field.
     choices: tuple[str, ...] | None = None
+    # What each choice is called in the UI, in the same order. Without it the stored
+    # value is shown, which reads fine for "Inkwell" and badly for "starttls".
+    choice_labels: tuple[str, ...] | None = None
     # A credential. The admin API never sends its value back, only whether one is
     # set, and saving it empty leaves it as it was (#5266).
     secret: bool = False
@@ -144,6 +147,7 @@ REGISTRY: list[SettingDef] = [
         "How the connection to the SMTP server is secured.",
         "Email",
         choices=("starttls", "tls", "none"),
+        choice_labels=("STARTTLS (usually port 587)", "TLS (usually port 465)", "None (unencrypted)"),
     ),
     SettingDef("smtp_username", "string", "", "SMTP username", "Leave empty if no sign-in is needed.", "Email"),
     SettingDef(
@@ -351,6 +355,7 @@ async def get_admin_settings(db) -> list[dict]:
                 "is_set": bool(value) if d.secret else None,
                 "secret": d.secret,
                 "choices": list(d.choices) if d.choices else None,
+                "choice_labels": list(d.choice_labels) if d.choice_labels else None,
                 "default": d.default,
                 "label": d.label,
                 "description": d.description,
diff --git a/src/inkwell/storage.py b/src/inkwell/storage.py
index 41c1437..c9ff0fb 100644
--- a/src/inkwell/storage.py
+++ b/src/inkwell/storage.py
@@ -1,7 +1,7 @@
 """How much attachment storage an account may use (#2939 §4).
 
 `max_attachment_mb` caps one file; this caps an account's total, so one account
-can't fill the volume. The limit is Settings → Attachments → `storage_quota_gb`;
+can't fill the volume. The limit is Settings → General → `storage_quota_gb`;
 0 turns it off. Admins have no limit, because the instance is theirs to fill.
 
 What counts is every attachment on every note the account owns, trashed notes
diff --git a/tests/test_settings.py b/tests/test_settings.py
index 9f2ac60..ca101b1 100644
--- a/tests/test_settings.py
+++ b/tests/test_settings.py
@@ -68,3 +68,11 @@ def test_the_public_address_is_an_http_url_without_a_trailing_slash():
     assert validate_updates({"public_url": ""}) == ({"public_url": ""}, None)
     clean, error = validate_updates({"public_url": "javascript:alert(1)"})
     assert clean == {} and error
+
+
+def test_choice_labels_name_every_choice():
+    """The UI pairs labels with choices by position, so a label list of another length
+    would put one choice's name on another."""
+    for d in REGISTRY:
+        if d.choice_labels is not None:
+            assert d.choices is not None and len(d.choice_labels) == len(d.choices), d.key