From f41061f85f7a4427546ea03ddcc6d5cfaccdf6d2 Mon Sep 17 00:00:00 2001
From: Brian Krabach <brkrabac@microsoft.com>
Date: Fri, 27 Mar 2026 18:02:29 -0700
Subject: [PATCH] fix: correct sidebar element IDs, add missing CSS classes,
 and align test mocks
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Three-part consistency defect identified in code review:

1. HTML: Four sidebar elements missing CSS class attributes so all CSS
   rules targeting .session-sidebar, .sidebar-toggle-btn,
   .sidebar-collapse-btn, and .sidebar-list applied to zero elements.
   Added class= attributes to all four.

2. JS: initSidebar(), toggleSidebar(), and bindSidebarClickAway() called
   $('sidebar') and $('collapse-btn') — neither ID exists in the DOM.
   Corrected to $('session-sidebar') and $('sidebar-collapse-btn').

3. Tests: initSidebar/toggleSidebar mocks matched the wrong IDs, masking
   the defect. Updated all six mock conditions to match corrected IDs.

4. Coverage gap: test_html_element_classes did not verify new sidebar
   elements carry CSS classes. Added four entries for session-sidebar,
   sidebar-toggle-btn, sidebar-collapse-btn, and sidebar-list — these
   entries would have caught Defect 1 on Task 1's test run.

All 136 JS + 167 Python tests pass. Zero regressions.
---
 coordinator/tests/test_frontend_html.py |  8 ++++++++
 frontend/app.js                         |  8 ++++----
 frontend/index.html                     |  8 ++++----
 frontend/tests/test_app.mjs             | 24 ++++++++++++------------
 4 files changed, 28 insertions(+), 20 deletions(-)

diff --git a/coordinator/tests/test_frontend_html.py b/coordinator/tests/test_frontend_html.py
index 0ee2d75..73f9739 100644
--- a/coordinator/tests/test_frontend_html.py
+++ b/coordinator/tests/test_frontend_html.py
@@ -247,6 +247,14 @@ def test_html_element_classes() -> None:
         ),
         ("session-pill-label", "session-pill__label", "needs max-width truncation"),
         ("session-pill-bell", "session-pill__bell", "needs amber var(--bell) color"),
+        (
+            "session-sidebar",
+            "session-sidebar",
+            "flex-column layout and collapse transition",
+        ),
+        ("sidebar-toggle-btn", "sidebar-toggle-btn", "36x36 bordered button styles"),
+        ("sidebar-collapse-btn", "sidebar-collapse-btn", "chevron button hover styles"),
+        ("sidebar-list", "sidebar-list", "flex:1 overflow-y:auto scroll container"),
     ]
     for el_id, expected_class, reason in cases:
         el = soup.find(id=el_id)
diff --git a/frontend/app.js b/frontend/app.js
index 20bad99..ef7d752 100644
--- a/frontend/app.js
+++ b/frontend/app.js
@@ -393,7 +393,7 @@ function initSidebar() {
     isOpen = window.innerWidth >= SIDEBAR_NARROW_THRESHOLD;
   }
 
-  const sidebar = $('sidebar');
+  const sidebar = $('session-sidebar');
   if (sidebar) {
     if (isOpen) {
       sidebar.classList.remove('sidebar--collapsed');
@@ -432,7 +432,7 @@ function toggleSidebar() {
   } catch (_) { /* blocked — ok */ }
 
   // Apply class
-  const sidebar = $('sidebar');
+  const sidebar = $('session-sidebar');
   if (sidebar) {
     if (isOpen) {
       sidebar.classList.remove('sidebar--collapsed');
@@ -442,7 +442,7 @@ function toggleSidebar() {
   }
 
   // Update collapse button text (‹ when open, › when closed)
-  const collapseBtn = $('collapse-btn');
+  const collapseBtn = $('sidebar-collapse-btn');
   if (collapseBtn) {
     collapseBtn.textContent = isOpen ? '\u2039' : '\u203a';
   }
@@ -461,7 +461,7 @@ function bindSidebarClickAway() {
   if (!container) return;
   container.addEventListener('click', () => {
     if (window.innerWidth >= SIDEBAR_NARROW_THRESHOLD) return;
-    const sidebar = $('sidebar');
+    const sidebar = $('session-sidebar');
     if (!sidebar) return;
     if (sidebar.classList.contains('sidebar--collapsed')) return;
     sidebar.classList.add('sidebar--collapsed');
diff --git a/frontend/index.html b/frontend/index.html
index 9c717b7..eb98212 100644
--- a/frontend/index.html
+++ b/frontend/index.html
@@ -30,17 +30,17 @@
   <div id="view-expanded" class="view hidden">
     <header class="expanded-header">
       <button id="back-btn" class="back-btn" aria-label="Back">&#8592;</button>
-      <button id="sidebar-toggle-btn" aria-label="Toggle session list">&#9776;</button>
+      <button id="sidebar-toggle-btn" class="sidebar-toggle-btn" aria-label="Toggle session list">&#9776;</button>
       <span id="expanded-session-name" class="expanded-session-name"></span>
       <button id="palette-trigger" class="palette-trigger" aria-label="Open command palette">&#8984;K</button>
     </header>
     <div class="view-body">
-      <div id="session-sidebar">
+      <div id="session-sidebar" class="session-sidebar">
         <div class="sidebar-header">
           <span class="sidebar-title">Sessions</span>
-          <button id="sidebar-collapse-btn" aria-label="Collapse session list">&#8249;</button>
+          <button id="sidebar-collapse-btn" class="sidebar-collapse-btn" aria-label="Collapse session list">&#8249;</button>
         </div>
-        <div id="sidebar-list"></div>
+        <div id="sidebar-list" class="sidebar-list"></div>
       </div>
       <div id="terminal-container" class="terminal-container"></div>
     </div>
diff --git a/frontend/tests/test_app.mjs b/frontend/tests/test_app.mjs
index 7c8b161..0933b98 100644
--- a/frontend/tests/test_app.mjs
+++ b/frontend/tests/test_app.mjs
@@ -1829,8 +1829,8 @@ test('initSidebar defaults to open (removes sidebar--collapsed) on wide screens
   const mockCollapseBtn = { textContent: '' };
   const origGetById = globalThis.document.getElementById;
   globalThis.document.getElementById = (id) => {
-    if (id === 'sidebar') return mockSidebar;
-    if (id === 'collapse-btn') return mockCollapseBtn;
+    if (id === 'session-sidebar') return mockSidebar;
+    if (id === 'sidebar-collapse-btn') return mockCollapseBtn;
     return null;
   };
 
@@ -1856,8 +1856,8 @@ test('initSidebar defaults to closed (adds sidebar--collapsed) on narrow screens
   const mockCollapseBtn = { textContent: '' };
   const origGetById = globalThis.document.getElementById;
   globalThis.document.getElementById = (id) => {
-    if (id === 'sidebar') return mockSidebar;
-    if (id === 'collapse-btn') return mockCollapseBtn;
+    if (id === 'session-sidebar') return mockSidebar;
+    if (id === 'sidebar-collapse-btn') return mockCollapseBtn;
     return null;
   };
 
@@ -1883,8 +1883,8 @@ test('initSidebar respects stored value true regardless of screen width — even
   const mockCollapseBtn = { textContent: '' };
   const origGetById = globalThis.document.getElementById;
   globalThis.document.getElementById = (id) => {
-    if (id === 'sidebar') return mockSidebar;
-    if (id === 'collapse-btn') return mockCollapseBtn;
+    if (id === 'session-sidebar') return mockSidebar;
+    if (id === 'sidebar-collapse-btn') return mockCollapseBtn;
     return null;
   };
 
@@ -1908,8 +1908,8 @@ test('toggleSidebar persists state to localStorage — from true toggles to fals
   const mockCollapseBtn = { textContent: '' };
   const origGetById = globalThis.document.getElementById;
   globalThis.document.getElementById = (id) => {
-    if (id === 'sidebar') return mockSidebar;
-    if (id === 'collapse-btn') return mockCollapseBtn;
+    if (id === 'session-sidebar') return mockSidebar;
+    if (id === 'sidebar-collapse-btn') return mockCollapseBtn;
     return null;
   };
 
@@ -1930,8 +1930,8 @@ test('toggleSidebar adds sidebar--collapsed class when closing (from open)', ()
   const mockCollapseBtn = { textContent: '' };
   const origGetById = globalThis.document.getElementById;
   globalThis.document.getElementById = (id) => {
-    if (id === 'sidebar') return mockSidebar;
-    if (id === 'collapse-btn') return mockCollapseBtn;
+    if (id === 'session-sidebar') return mockSidebar;
+    if (id === 'sidebar-collapse-btn') return mockCollapseBtn;
     return null;
   };
 
@@ -1952,8 +1952,8 @@ test('toggleSidebar removes sidebar--collapsed class when opening (from closed)
   const mockCollapseBtn = { textContent: '' };
   const origGetById = globalThis.document.getElementById;
   globalThis.document.getElementById = (id) => {
-    if (id === 'sidebar') return mockSidebar;
-    if (id === 'collapse-btn') return mockCollapseBtn;
+    if (id === 'session-sidebar') return mockSidebar;
+    if (id === 'sidebar-collapse-btn') return mockCollapseBtn;
     return null;
   };