Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions docs/guides/architecture.md
Original file line number Diff line number Diff line change
Expand Up @@ -297,6 +297,16 @@ The helper is wired into every icon render site: `sidebar-projects.js`, `app-hom

`app-home-hub.js` previously interpolated `proj.icon` directly into `innerHTML`. That site now uses DOM API with `renderProjectIcon`, eliminating the stored-XSS vector.

## Server Settings mobile parity (lr-87ca)

**Rule:** any Server Settings section or affordance that ships on desktop MUST ship with first-class mobile parity in the same change. "Reachable via the command palette" or "reachable via a synthetic click on a hidden desktop button" is NOT parity — it is a discoverability defect.

**Root cause this rule fixes:** `#server-settings-btn` lives inside `#top-bar`, which is `display:none` at `<=768px` (`lib/public/css/title-bar.css`). Before lr-87ca, the mobile "More" sheet (`lib/public/modules/sidebar-mobile.js`) opened Server Settings by synthetic-clicking that hidden button (`document.getElementById("server-settings-btn").click()`), which only worked because the click handler still fired despite the element being invisible. This left every Server Settings section (Storage, Custom Icons, etc.) technically reachable but not a first-class, discoverable nav path.

**Fix pattern:** `server-settings.js` exports `openServerSettings()` — a public entry point independent of the desktop button element — mirroring the existing `openProjectSettings()` pattern already used by Project Settings. `sidebar-mobile.js`'s "More" sheet imports and calls `openServerSettings()` directly instead of dispatching a synthetic click. Once the panel is open, per-section navigation is already shared code between desktop and mobile: the `.server-settings-nav-items` sidebar list is desktop-only (hidden at `<=768px`), replaced by `#settings-nav-pill` → the settings command palette (`openSettingsPalette()`), which lists every entry in `SETTINGS_SECTIONS` (the same registry used to render the desktop sidebar) — so no separate mobile section audit is needed as long as new sections are added to `SETTINGS_SECTIONS`.

**When adding a new Server Settings section:** add it to `SETTINGS_SECTIONS` in `server-settings.js`, add the corresponding nav item + `.server-settings-section` panel markup in `index.html`, and verify it renders and functions at `<=768px` (mobile nav pill → palette → section). Do not add a desktop-only entry point (e.g. wiring a new button that only lives in `#top-bar`).

## See Also

- [MODULE_MAP.md](./MODULE_MAP.md) — where every module lives and what it owns
Expand Down
8 changes: 8 additions & 0 deletions lib/public/modules/server-settings.js
Original file line number Diff line number Diff line change
Expand Up @@ -558,6 +558,14 @@ function openSettings() {
statsTimer = setInterval(requestStats, 5000);
}

// Public entry point for callers outside this module (e.g. the mobile More
// sheet — lr-87ca) that need to open Server Settings without depending on
// the desktop top-bar button, which is display:none on mobile.
export function openServerSettings() {
if (!settingsEl || !settingsBtn) return;
openSettings();
}

function resetRestartButton() {
var btn = document.getElementById("settings-restart-btn");
var errorEl = document.getElementById("settings-restart-error");
Expand Down
8 changes: 6 additions & 2 deletions lib/public/modules/sidebar-mobile.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { getCurrentTheme, getChatLayout, setChatLayout, toggleDarkMode, getBrand
import { openCommandPalette } from './command-palette.js';
import { openAgentPicker, openAgentPickerForPreferred } from './agent-picker.js';
import { openProjectSettings } from './project-settings.js';
import { openServerSettings } from './server-settings.js';
import {
getCachedSessions,
getDateGroup,
Expand Down Expand Up @@ -1128,8 +1129,11 @@ function renderSheetMore(listEl) {
});
addItem("settings", "Server Settings", function () {
closeMobileSheet();
var btn = document.getElementById("server-settings-btn");
if (btn) setTimeout(function () { btn.click(); }, 250);
// lr-87ca: call the shared open function directly rather than
// synthetic-clicking the desktop-only #server-settings-btn (which lives
// inside #top-bar, display:none on mobile). This is a first-class nav
// path, matching the Project Settings pattern above.
setTimeout(function () { openServerSettings(); }, 250);
});

addDivider();
Expand Down
86 changes: 86 additions & 0 deletions test/server-settings-mobile-parity-lr-87ca.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
// server-settings-mobile-parity-lr-87ca.test.js — lr-87ca regression coverage.
//
// Root cause (andy, 2026-07-01): #server-settings-btn lives inside #top-bar,
// which is display:none at <=768px (lib/public/css/title-bar.css). Before
// this fix, the mobile "More" sheet opened Server Settings by
// synthetic-clicking that hidden button — functional, but not a first-class
// discoverable nav path (the button never appears in the mobile UI at all).
//
// Fix: server-settings.js exports openServerSettings(), a public entry point
// independent of the desktop button element (mirrors the existing
// openProjectSettings() pattern already used by Project Settings).
// sidebar-mobile.js imports it and calls it directly instead of dispatching
// a synthetic click.
//
// These are source-text regression checks (no DOM harness required — both
// modules perform their DOM work inside functions, not at import time,
// consistent with the project's existing xss-escape.test.js convention).

"use strict";

var test = require("node:test");
var assert = require("node:assert/strict");
var fs = require("fs");
var path = require("path");

var SERVER_SETTINGS_SRC = fs.readFileSync(
path.join(__dirname, "../lib/public/modules/server-settings.js"),
"utf8"
);
var SIDEBAR_MOBILE_SRC = fs.readFileSync(
path.join(__dirname, "../lib/public/modules/sidebar-mobile.js"),
"utf8"
);

test("server-settings.js exports openServerSettings as a public entry point", () => {
assert.match(
SERVER_SETTINGS_SRC,
/export function openServerSettings\s*\(/,
"openServerSettings must be an exported function so callers outside this module can open the panel without the desktop button"
);
});

test("sidebar-mobile.js imports openServerSettings from server-settings.js", () => {
assert.match(
SIDEBAR_MOBILE_SRC,
/import\s*\{\s*openServerSettings\s*\}\s*from\s*['"]\.\/server-settings\.js['"]/,
"the mobile More sheet must import the real open function, not rely on a global lookup"
);
});

test("sidebar-mobile.js Server Settings entry calls openServerSettings(), not a synthetic click", () => {
// Isolate the "Server Settings" addItem() call block so this assertion is
// scoped to the regression site rather than the whole file.
var idx = SIDEBAR_MOBILE_SRC.indexOf('addItem("settings", "Server Settings"');
assert.ok(idx !== -1, 'expected addItem("settings", "Server Settings", ...) call to exist');
var block = SIDEBAR_MOBILE_SRC.slice(idx, idx + 500);

assert.match(
block,
/openServerSettings\s*\(\s*\)/,
"Server Settings mobile entry must call openServerSettings() directly"
);
assert.doesNotMatch(
block,
/getElementById\(["']server-settings-btn["']\)/,
"Server Settings mobile entry must not look up the desktop-only #server-settings-btn (hidden at <=768px, lr-87ca root cause)"
);
assert.doesNotMatch(
block,
/\.click\(\)/,
"Server Settings mobile entry must not dispatch a synthetic click — that is the discoverability defect lr-87ca fixes"
);
});

test("SETTINGS_SECTIONS registry includes custom-icons (lr-d1d9 section stays reachable via the mobile palette)", () => {
var idx = SERVER_SETTINGS_SRC.indexOf("var SETTINGS_SECTIONS");
assert.ok(idx !== -1, "expected SETTINGS_SECTIONS array to exist");
var arrayEnd = SERVER_SETTINGS_SRC.indexOf("];", idx);
var block = SERVER_SETTINGS_SRC.slice(idx, arrayEnd);

assert.match(
block,
/section:\s*['"]custom-icons['"]/,
"custom-icons must stay registered in SETTINGS_SECTIONS — this is the shared registry that drives both the desktop nav sidebar and the mobile settings palette (openSettingsPalette), so removing it here would silently break mobile discoverability of Custom Icons even though the entry-point fix above is correct"
);
});
Loading