refactor: rewrite sidebar functions to use server-side settings

- Remove SIDEBAR_KEY constant (was an undefined variable bug)
- Rewrite initSidebar() to read sidebarOpen from _serverSettings
- Rewrite toggleSidebar() to derive state from DOM class and persist
  to server via patchServerSetting
- Rewrite bindSidebarClickAway() to persist collapsed state via
  patchServerSetting instead of localStorage
- Add 7 tests verifying SIDEBAR_KEY removal and _serverSettings usage
This commit is contained in:
Brian Krabach
2026-04-08 11:29:55 -07:00
parent ecc5e6d42a
commit 5da5fa8a45
2 changed files with 141 additions and 55 deletions
+25 -45
View File
@@ -598,24 +598,24 @@ const SIDEBAR_NARROW_THRESHOLD = 960;
/** /**
* Initialise sidebar open/closed state on page load. * Initialise sidebar open/closed state on page load.
* Reads muxplex.sidebarOpen from localStorage (JSON.parse with try/catch). * Reads sidebarOpen from _serverSettings cache.
* Defaults to open on wide screens (innerWidth >= 960) when no stored value. * Defaults to open on wide screens (innerWidth >= 960) when no stored value.
* Applies sidebar--collapsed class accordingly and persists the initial state. * Applies sidebar--collapsed class accordingly and persists the initial state.
*/ */
function initSidebar() { function initSidebar() {
let isOpen; var stored = _serverSettings ? _serverSettings.sidebarOpen : null;
try { var isOpen;
const stored = localStorage.getItem(SIDEBAR_KEY);
if (stored !== null) { if (stored !== null && stored !== undefined) {
isOpen = JSON.parse(stored); isOpen = !!stored;
} else { } else {
isOpen = window.innerWidth >= SIDEBAR_NARROW_THRESHOLD; isOpen = window.innerWidth >= SIDEBAR_NARROW_THRESHOLD;
} // Persist the auto-detected value (fire-and-forget)
} catch (_) { if (_serverSettings) _serverSettings.sidebarOpen = isOpen;
isOpen = window.innerWidth >= SIDEBAR_NARROW_THRESHOLD; patchServerSetting('sidebarOpen', isOpen);
} }
const sidebar = $('session-sidebar'); var sidebar = $('session-sidebar');
if (sidebar) { if (sidebar) {
if (isOpen) { if (isOpen) {
sidebar.classList.remove('sidebar--collapsed'); sidebar.classList.remove('sidebar--collapsed');
@@ -623,51 +623,32 @@ function initSidebar() {
sidebar.classList.add('sidebar--collapsed'); sidebar.classList.add('sidebar--collapsed');
} }
} }
// Persist initial state
try {
localStorage.setItem(SIDEBAR_KEY, JSON.stringify(isOpen));
} catch (_) { /* blocked — ok */ }
} }
/** /**
* Toggle the sidebar open/closed state. * Toggle the sidebar open/closed state.
* Reads current state from localStorage, inverts it, persists, applies * Derives current state from DOM class, inverts it, persists to server,
* sidebar--collapsed class, and updates the collapse button text. * applies sidebar--collapsed class, and updates the collapse button text.
* Button shows when open, when closed. * Button shows when open, when closed.
*/ */
function toggleSidebar() { function toggleSidebar() {
let isOpen; var sidebar = $('session-sidebar');
try { if (!sidebar) return;
const stored = localStorage.getItem(SIDEBAR_KEY);
isOpen = stored !== null ? JSON.parse(stored) : true;
} catch (_) {
isOpen = true;
}
// Invert state var isOpen = !sidebar.classList.contains('sidebar--collapsed');
isOpen = !isOpen; isOpen = !isOpen;
// Persist
try {
localStorage.setItem(SIDEBAR_KEY, JSON.stringify(isOpen));
} catch (_) { /* blocked — ok */ }
// Apply class
const sidebar = $('session-sidebar');
if (sidebar) {
if (isOpen) { if (isOpen) {
sidebar.classList.remove('sidebar--collapsed'); sidebar.classList.remove('sidebar--collapsed');
} else { } else {
sidebar.classList.add('sidebar--collapsed'); sidebar.classList.add('sidebar--collapsed');
} }
}
// Update collapse button text ( when open, when closed) if (_serverSettings) _serverSettings.sidebarOpen = isOpen;
const collapseBtn = $('sidebar-collapse-btn'); patchServerSetting('sidebarOpen', isOpen);
if (collapseBtn) {
collapseBtn.textContent = isOpen ? '\u2039' : '\u203a'; var collapseBtn = $('sidebar-collapse-btn');
} if (collapseBtn) collapseBtn.textContent = isOpen ? '\u2039' : '\u203a';
} }
/** /**
@@ -679,17 +660,16 @@ function toggleSidebar() {
* - the sidebar is already collapsed * - the sidebar is already collapsed
*/ */
function bindSidebarClickAway() { function bindSidebarClickAway() {
const container = $('terminal-container'); var container = $('terminal-container');
if (!container) return; if (!container) return;
container.addEventListener('click', () => { container.addEventListener('click', function() {
if (window.innerWidth >= SIDEBAR_NARROW_THRESHOLD) return; if (window.innerWidth >= SIDEBAR_NARROW_THRESHOLD) return;
const sidebar = $('session-sidebar'); var sidebar = $('session-sidebar');
if (!sidebar) return; if (!sidebar) return;
if (sidebar.classList.contains('sidebar--collapsed')) return; if (sidebar.classList.contains('sidebar--collapsed')) return;
sidebar.classList.add('sidebar--collapsed'); sidebar.classList.add('sidebar--collapsed');
try { if (_serverSettings) _serverSettings.sidebarOpen = false;
localStorage.setItem(SIDEBAR_KEY, JSON.stringify(false)); patchServerSetting('sidebarOpen', false);
} catch (_) { /* blocked — ok */ }
}); });
} }
+111 -5
View File
@@ -344,7 +344,9 @@ def test_get_display_settings_reads_server_settings() -> None:
) )
assert match, "getDisplaySettings function not found" assert match, "getDisplaySettings function not found"
body = match.group(1) body = match.group(1)
assert "_serverSettings" in body, "getDisplaySettings must read from _serverSettings" assert "_serverSettings" in body, (
"getDisplaySettings must read from _serverSettings"
)
assert "localStorage" not in body, ( assert "localStorage" not in body, (
"getDisplaySettings must not use localStorage — display settings are server-side" "getDisplaySettings must not use localStorage — display settings are server-side"
) )
@@ -429,7 +431,9 @@ def test_get_display_settings_reads_from_server_settings() -> None:
) )
assert match, "getDisplaySettings function not found" assert match, "getDisplaySettings function not found"
body = match.group(1) body = match.group(1)
assert "_serverSettings" in body, "getDisplaySettings must read from _serverSettings" assert "_serverSettings" in body, (
"getDisplaySettings must read from _serverSettings"
)
assert "localStorage" not in body, ( assert "localStorage" not in body, (
"getDisplaySettings must not use localStorage — display settings are server-side" "getDisplaySettings must not use localStorage — display settings are server-side"
) )
@@ -492,9 +496,7 @@ def test_on_display_setting_change_catches_errors() -> None:
) )
assert match, "onDisplaySettingChange function not found" assert match, "onDisplaySettingChange function not found"
body = match.group(1) body = match.group(1)
assert ".catch" in body, ( assert ".catch" in body, "onDisplaySettingChange must handle errors via .catch()"
"onDisplaySettingChange must handle errors via .catch()"
)
# ── openSettings implementation ─────────────────────────────────────────────── # ── openSettings implementation ───────────────────────────────────────────────
@@ -2597,3 +2599,107 @@ def test_open_session_bell_clear_is_fire_and_forget() -> None:
assert "await" not in line, ( assert "await" not in line, (
f"openSession bell-clear POST must NOT be awaited (fire-and-forget): {line.strip()}" f"openSession bell-clear POST must NOT be awaited (fire-and-forget): {line.strip()}"
) )
# ─── Task 4: sidebar functions use server-side settings ──────────────────────
def test_no_sidebar_key_constant() -> None:
"""SIDEBAR_KEY constant must be removed — sidebar state moves to _serverSettings."""
assert "SIDEBAR_KEY" not in _JS, (
"SIDEBAR_KEY constant must be removed from app.js; "
"sidebar open/closed state is now stored in _serverSettings.sidebarOpen"
)
def test_init_sidebar_reads_server_settings() -> None:
"""initSidebar must read sidebarOpen from _serverSettings, not localStorage."""
match = re.search(
r"function initSidebar\s*\(\s*\)\s*\{(.*?)(?=\nfunction |\n// ─|\n/\*\*)",
_JS,
re.DOTALL,
)
assert match, "initSidebar function not found"
body = match.group(1)
assert "_serverSettings" in body, (
"initSidebar must read sidebarOpen from _serverSettings"
)
assert "localStorage" not in body, (
"initSidebar must not use localStorage — sidebar state is now server-side"
)
def test_init_sidebar_calls_patch_server_setting() -> None:
"""initSidebar must call patchServerSetting to persist the auto-detected state."""
match = re.search(
r"function initSidebar\s*\(\s*\)\s*\{(.*?)(?=\nfunction |\n// ─|\n/\*\*)",
_JS,
re.DOTALL,
)
assert match, "initSidebar function not found"
body = match.group(1)
assert "patchServerSetting" in body, (
"initSidebar must call patchServerSetting to persist the auto-detected sidebar state"
)
def test_toggle_sidebar_reads_server_settings() -> None:
"""toggleSidebar must derive state from the DOM class, not from localStorage."""
match = re.search(
r"function toggleSidebar\s*\(\s*\)\s*\{(.*?)(?=\nfunction |\n// ─|\n/\*\*)",
_JS,
re.DOTALL,
)
assert match, "toggleSidebar function not found"
body = match.group(1)
assert "_serverSettings" in body, (
"toggleSidebar must write new state to _serverSettings"
)
assert "localStorage" not in body, (
"toggleSidebar must not use localStorage — sidebar state is now server-side"
)
def test_toggle_sidebar_calls_patch_server_setting() -> None:
"""toggleSidebar must call patchServerSetting to persist the toggled state."""
match = re.search(
r"function toggleSidebar\s*\(\s*\)\s*\{(.*?)(?=\nfunction |\n// ─|\n/\*\*)",
_JS,
re.DOTALL,
)
assert match, "toggleSidebar function not found"
body = match.group(1)
assert "patchServerSetting" in body, (
"toggleSidebar must call patchServerSetting to persist the toggled sidebar state"
)
def test_bind_sidebar_click_away_uses_server_settings() -> None:
"""bindSidebarClickAway must write false to _serverSettings on collapse."""
match = re.search(
r"function bindSidebarClickAway\s*\(\s*\)\s*\{(.*?)(?=\nfunction |\n// ─|\n/\*\*)",
_JS,
re.DOTALL,
)
assert match, "bindSidebarClickAway function not found"
body = match.group(1)
assert "_serverSettings" in body, (
"bindSidebarClickAway must write false to _serverSettings.sidebarOpen on collapse"
)
assert "localStorage" not in body, (
"bindSidebarClickAway must not use localStorage — sidebar state is now server-side"
)
def test_bind_sidebar_click_away_calls_patch_server_setting() -> None:
"""bindSidebarClickAway must call patchServerSetting to persist collapsed state."""
match = re.search(
r"function bindSidebarClickAway\s*\(\s*\)\s*\{(.*?)(?=\nfunction |\n// ─|\n/\*\*)",
_JS,
re.DOTALL,
)
assert match, "bindSidebarClickAway function not found"
body = match.group(1)
assert "patchServerSetting" in body, (
"bindSidebarClickAway must call patchServerSetting to persist collapsed state"
)