emrg: GUI sidebar — remove history session list + keyboard nav (rant 2026-08-21T11:44:52) - #911
Merged
Merged
Conversation
added 2 commits
August 21, 2026 12:42
argszero
commented
Aug 21, 2026
argszero
left a comment
OwnerAuthor
There was a problem hiding this comment.
✅ LGTM — cycle
Fresh review of the sidebar rework (rant 2026-08-21T11:44:52):
Sidebar.render()(history#conv-list) removed entirely, plus its 5 app.js call sites (boot, session-not-found branch, refreshSessions, sessions/sessions_list broadcast handlers);state.sessionsretained as internal state for switching/title bar.- Keyboard nav removed:
initKeyboard/clearFocus/kbd-focusgone from sidebar.js;.conv-item.kbd-focusCSS rule removed;<nav id="conv-list">removed from index.html;highlight()now targetsopen-sessionsonly. - Open-sessions persistence untouched (gui_state.json + restoreOpenSessions already cover restarts — no gap found).
- Tests: boot test now asserts
state.sessions; 2 keyboard-nav tests replaced by a negative test asserting the removal; scoped-highlight test updated for single-list sidebar. Agent.md breakdown synced (merged count 255: renderer smoke 126, integration 8). - Master merged in to resolve the Agent.md conflict with #910; verified on merged head: GUI npm test 255 (247 pass + 8 skipped), Python pytest 987 passed + 1 skipped incl. doc-count guards, import + CLI OK. CI test + test-windows PASS.
1/3.
argszero
commented
Aug 21, 2026
argszero
left a comment
OwnerAuthor
There was a problem hiding this comment.
✅ LGTM — cycle
Re-review (head c884c2a unchanged since previous review):
Sidebar.render()(history#conv-list) removed entirely, along withcwdProjectName,initKeyboard,clearFocus,_keyHandler,_focusIdxand the module-levelinit();highlight()now targetsopen-sessionsonly and the exposed API is just{ renderOpenSessions, highlight }.- All 5
Sidebar.render(...)call sites removed from app.js (boot, switchSession session-not-found, refreshSessions, sessions/sessions_list broadcast handlers);state.sessionsretained for switching/title bar. <nav id="conv-list">removed from index.html;.conv-item.kbd-focusCSS removed; ELEMENT_IDS cleaned.- Tests: boot test asserts
state.sessions; two keyboard-nav tests replaced by a negative test asserting the removal (conv-list / kbd-focus / initKeyboard / clearFocus absent from sidebar.js, no conv-list nav in index.html); scoped-highlight test updated for single-list sidebar. Agent.md merged count 255 (renderer smoke 126, integration 8). - Verified on merged head: GUI npm test 255 (247 pass + 8 skipped), Python pytest 987 passed + 1 skipped incl. doc-count guards, import + CLI OK. CI test + test-windows PASS.
2/3.
pm25coder
commented
Aug 21, 2026
Collaborator
Independent test report (Contributor, pm25coder): |
argszero
commented
Aug 21, 2026
argszero
left a comment
OwnerAuthor
There was a problem hiding this comment.
✅ LGTM — cycle
3rd consecutive review (head c884c2a, MERGEABLE/CLEAN, CI test + test-windows PASS):
- Fresh local verification on the branch: GUI npm test 255 total (247 pass / 0 fail / 8 skipped — matches Agent.md update 256→255), pytest 987 passed + 1 skipped, import + CLI OK.
- Sidebar.render() (history #conv-list) removed entirely with its keyboard-nav machinery (initKeyboard/clearFocus/_keyHandler/_focusIdx/cwdProjectName); 5 app.js call sites cleaned; index.html drops the conv-list nav; components.css drops the kbd-focus style; highlight() now targets only the open-sessions area — consistent with the new sidebar design (rant 11:44:52).
3/3.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes rant
2026-08-21T11:44:52— GUI sidebar rework:Remove the history session list: the sidebar now shows only the "Open Sessions" list.
Sidebar.render()(which rendered the#conv-listhistory list) is removed entirely along with all 5 call sites inapp.js(boot init, switchSession session-not-found branch, refreshSessions, and thesessions/sessions_listbroadcast handlers —state.sessionsis still kept as internal state for session switching and the title bar). The<nav id="conv-list">element is removed fromindex.html.Remove keyboard navigation:
initKeyboard/clearFocus/kbd-focusare gone fromsidebar.js(the ↑↓/Enter nav was bound to the removed history list), and the.conv-item.kbd-focusCSS rule is removed.Open-sessions persistence: unchanged — the existing mechanism (gui_state.json +
restoreOpenSessions+ rendererinit.open_sessions) already restores open sessions across restarts; no gap found.Changes
emrg/gui/renderer/js/sidebar.js— droprender(),cwdProjectName, keyboard-nav block;highlight()targetsopen-sessionsonlyemrg/gui/renderer/js/app.js— remove 5Sidebar.render(...)call sites (state.sessions retained)emrg/gui/renderer/index.html— remove<nav id="conv-list">emrg/gui/renderer/css/components.css— remove.conv-item.kbd-focusruleemrg/gui/test/renderer.smoke.test.js— boot test now assertsstate.sessions; 2 keyboard-nav tests replaced by a negative test asserting the removal; scoped-highlight test updated for single-list sidebarAgent.md— GUI breakdown synced (renderer smoke 126→125, integration 7→8 real node count)Verification
npm test— 254 tests, 246 pass, 8 skipped (live daemon owns fixed port), 0 failuv run pytest tests/— 987 passed + 1 skipped (incl. doc-count guards)python -m emrg --help+ import OK