refactor: code quality improvements for task-7 settings effects

- Extract getVisibleSessions() helper to consolidate hidden-session
  filter used by renderGrid() and renderSidebar() (DRY reduction)
- Add comment to terminal.js magic literal 600 noting it matches
  MOBILE_THRESHOLD in app.js
- Move TERMINAL_JS_PATH and _TERMINAL_JS module fixtures to the top
  of the test file alongside JS_PATH and _JS for consistency
- Strengthen test_render_grid_uses_visible_for_empty_state_check to
  assert visible.length specifically, distinguishing it from
  test_render_grid_creates_visible_array
- Update renderGrid/renderSidebar filter tests to assert getVisibleSessions()
  call; add test_get_visible_sessions_filters_hidden_sessions to cover
  the extracted helper directly
This commit is contained in:
Brian Krabach
2026-03-30 02:28:54 -07:00
parent 142c68d644
commit 851036bac0
18 changed files with 47 additions and 28 deletions
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
+13 -6
View File
@@ -448,6 +448,17 @@ function buildSidebarHTML(session, currentSession) {
); );
} }
/**
* Returns sessions with hidden session names removed.
* Consolidates the hidden-session filter used by all render paths.
* @param {object[]} sessions
* @returns {object[]}
*/
function getVisibleSessions(sessions) {
const hidden = (_serverSettings && _serverSettings.hidden_sessions) || [];
return (sessions || []).filter((s) => !hidden.includes(s.name));
}
/** /**
* Render the session sidebar list. Only renders in fullscreen view. * Render the session sidebar list. Only renders in fullscreen view.
* Shows empty state when no sessions exist. * Shows empty state when no sessions exist.
@@ -461,9 +472,7 @@ function renderSidebar(sessions, currentSession) {
const list = $('sidebar-list'); const list = $('sidebar-list');
if (!list) return; if (!list) return;
// Filter hidden sessions const visible = getVisibleSessions(sessions);
const hiddenSessions = (_serverSettings && _serverSettings.hidden_sessions) || [];
const visible = (sessions || []).filter((s) => !hiddenSessions.includes(s.name));
if (visible.length === 0) { if (visible.length === 0) {
list.innerHTML = '<div class="sidebar-empty">No sessions</div>'; list.innerHTML = '<div class="sidebar-empty">No sessions</div>';
@@ -594,9 +603,7 @@ function renderGrid(sessions) {
const grid = $('session-grid'); const grid = $('session-grid');
const emptyState = $('empty-state'); const emptyState = $('empty-state');
// Filter hidden sessions const visible = getVisibleSessions(sessions);
const hiddenSessions = (_serverSettings && _serverSettings.hidden_sessions) || [];
const visible = (sessions || []).filter((s) => !hiddenSessions.includes(s.name));
if (visible.length === 0) { if (visible.length === 0) {
if (grid) grid.innerHTML = ''; if (grid) grid.innerHTML = '';
+1 -1
View File
@@ -150,7 +150,7 @@ function createTerminal() {
} }
} catch (_) { /* use default 14 */ } } catch (_) { /* use default 14 */ }
const mobile = window.innerWidth < 600; const mobile = window.innerWidth < 600; // matches MOBILE_THRESHOLD in app.js
const fontSize = mobile ? Math.min(storedFontSize, 12) : storedFontSize; const fontSize = mobile ? Math.min(storedFontSize, 12) : storedFontSize;
_term = new window.Terminal({ _term = new window.Terminal({
+33 -21
View File
@@ -4,9 +4,11 @@ import pathlib
import re import re
JS_PATH = pathlib.Path(__file__).parent.parent / "frontend" / "app.js" JS_PATH = pathlib.Path(__file__).parent.parent / "frontend" / "app.js"
TERMINAL_JS_PATH = pathlib.Path(__file__).parent.parent / "frontend" / "terminal.js"
# Read once per module — tests are read-only so sharing is safe. # Read once per module — tests are read-only so sharing is safe.
_JS: str = JS_PATH.read_text() _JS: str = JS_PATH.read_text()
_TERMINAL_JS: str = TERMINAL_JS_PATH.read_text()
# ── Palette state variables must be removed ────────────────────────────────── # ── Palette state variables must be removed ──────────────────────────────────
@@ -2050,10 +2052,6 @@ def test_js_show_fab_session_input_uses_factory() -> None:
# ─── Task 7: Apply settings effects (task-7-apply-settings-effects) ────────── # ─── Task 7: Apply settings effects (task-7-apply-settings-effects) ──────────
TERMINAL_JS_PATH = pathlib.Path(__file__).parent.parent / "frontend" / "terminal.js"
_TERMINAL_JS: str = TERMINAL_JS_PATH.read_text()
# ── terminal.js: createTerminal reads font size from localStorage ───────────── # ── terminal.js: createTerminal reads font size from localStorage ─────────────
@@ -2136,11 +2134,31 @@ def test_create_terminal_has_default_font_size_14() -> None:
) )
# ── app.js: getVisibleSessions helper filters hidden sessions ─────────────────
def test_get_visible_sessions_filters_hidden_sessions() -> None:
"""getVisibleSessions() must filter sessions using _serverSettings.hidden_sessions."""
match = re.search(
r"function getVisibleSessions\s*\(\w+\)\s*\{(.*?)(?=\n(?:function|//|window\.))",
_JS,
re.DOTALL,
)
assert match, "getVisibleSessions function not found in app.js"
body = match.group(1)
assert "hidden_sessions" in body, (
"getVisibleSessions must filter sessions using _serverSettings.hidden_sessions"
)
assert "_serverSettings" in body, (
"getVisibleSessions must reference _serverSettings to access hidden_sessions"
)
# ── app.js: renderGrid filters hidden sessions ──────────────────────────────── # ── app.js: renderGrid filters hidden sessions ────────────────────────────────
def test_render_grid_filters_hidden_sessions() -> None: def test_render_grid_filters_hidden_sessions() -> None:
"""renderGrid() must filter out hidden sessions using _serverSettings.hidden_sessions.""" """renderGrid() must filter out hidden sessions via getVisibleSessions()."""
match = re.search( match = re.search(
r"function renderGrid\s*\(\w+\)\s*\{(.*?)(?=\n(?:function|//|window\.))", r"function renderGrid\s*\(\w+\)\s*\{(.*?)(?=\n(?:function|//|window\.))",
_JS, _JS,
@@ -2148,11 +2166,8 @@ def test_render_grid_filters_hidden_sessions() -> None:
) )
assert match, "renderGrid function not found in app.js" assert match, "renderGrid function not found in app.js"
body = match.group(1) body = match.group(1)
assert "hidden_sessions" in body, ( assert "getVisibleSessions" in body, (
"renderGrid must filter sessions using _serverSettings.hidden_sessions" "renderGrid must call getVisibleSessions() to filter hidden sessions"
)
assert "_serverSettings" in body, (
"renderGrid must reference _serverSettings to access hidden_sessions"
) )
@@ -2171,7 +2186,7 @@ def test_render_grid_creates_visible_array() -> None:
def test_render_grid_uses_visible_for_empty_state_check() -> None: def test_render_grid_uses_visible_for_empty_state_check() -> None:
"""renderGrid() must use visible array (not sessions) for empty-state check.""" """renderGrid() must use visible.length (not sessions.length) for empty-state check."""
match = re.search( match = re.search(
r"function renderGrid\s*\(\w+\)\s*\{(.*?)(?=\n(?:function|//|window\.))", r"function renderGrid\s*\(\w+\)\s*\{(.*?)(?=\n(?:function|//|window\.))",
_JS, _JS,
@@ -2179,10 +2194,10 @@ def test_render_grid_uses_visible_for_empty_state_check() -> None:
) )
assert match, "renderGrid function not found in app.js" assert match, "renderGrid function not found in app.js"
body = match.group(1) body = match.group(1)
# After filtering hidden sessions, the empty state check should use visible.length, # The empty-state guard must check visible.length, not sessions.length, so hidden
# not sessions.length. Check that visible is used before the empty state handling. # sessions do not trigger the "no sessions" state.
assert "visible" in body, ( assert "visible.length" in body, (
"renderGrid must use 'visible' array for empty-state check" "renderGrid must use 'visible.length' for empty-state check, not sessions.length"
) )
@@ -2221,7 +2236,7 @@ def test_render_grid_reads_sort_order_from_server_settings() -> None:
def test_render_sidebar_filters_hidden_sessions() -> None: def test_render_sidebar_filters_hidden_sessions() -> None:
"""renderSidebar() must filter out hidden sessions using _serverSettings.hidden_sessions.""" """renderSidebar() must filter out hidden sessions via getVisibleSessions()."""
match = re.search( match = re.search(
r"function renderSidebar\s*\(\w+,\s*\w+\)\s*\{(.*?)(?=\nconst SIDEBAR_KEY|function |\n// ─)", r"function renderSidebar\s*\(\w+,\s*\w+\)\s*\{(.*?)(?=\nconst SIDEBAR_KEY|function |\n// ─)",
_JS, _JS,
@@ -2229,11 +2244,8 @@ def test_render_sidebar_filters_hidden_sessions() -> None:
) )
assert match, "renderSidebar function not found in app.js" assert match, "renderSidebar function not found in app.js"
body = match.group(1) body = match.group(1)
assert "hidden_sessions" in body, ( assert "getVisibleSessions" in body, (
"renderSidebar must filter sessions using _serverSettings.hidden_sessions" "renderSidebar must call getVisibleSessions() to filter hidden sessions"
)
assert "_serverSettings" in body, (
"renderSidebar must reference _serverSettings to access hidden_sessions"
) )