emrg: GUI file browser — chevron indicators + tree scroll fix - #769
Conversation
…026-08-13T20:58:57)
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle (1/3)
Full-diff review (actual PR diff = merge-base..head, 4 files):
- file-tree.js: chevronRight/chevronDown ICONs (correct VS Code ▸/▾ paths), iconSvg(d, cls) optional class param (backward-compatible), chevronFor() maps expanded state, dir rows + root row prepend .ft-chevron, toggleDir syncs chevron innerHTML on expand/collapse; file rows get no chevron. Clean.
- layout.css: .ft-chevron (flex:none, 14px, margin-right 2px, fill currentColor) + the key fix —
.ft-row { flex-shrink: 0 }. Verified the cascade: .result-files is a flex column (via .result-pane.active) with overflow-y:auto; .ft-row items default flex-shrink:1 + overflow:hidden → min-height:auto resolves to 0 → whole tree compresses → scrollHeight==clientHeight, never scrolls. flex-shrink:0 prevents the compression. Correct root cause + fix (matches the #733-era.result-files min-height:0scrollbar intent). - renderer.smoke.test.js: +1 P3.5 test — root chevron ▾ (M3 6l5 5 5-5z), collapsed dir ▸ (M6 3l5 5-5 5z), click → ▾, file rows no chevron. Discriminates the chevron behavior in both states.
- Agent.md: GUI 237→238 (renderer smoke 115→116) synced.
Note: branch base is pre-#768 (3dbd600) — the reversal lines only appear when diffing against new master; GitHub's squash diff (merge-base..head) is exactly these 4 files, so merge applies cleanly. CI test-windows PASS (test pending). Approving.
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle (2/3)
Fresh re-verification this cycle (head f37ecc9 unchanged since first review):
- file-tree.js: chevronRight/chevronDown ICONs (correct VS Code ▸/▾ paths), iconSvg(d, cls) keeps backward compat, chevronFor() reads the same
expandedMap that drives dirOpen/dirClosed — chevron state can never desync from folder state; toggleDir syncs both icons on expand/collapse; file rows get no chevron; root row (default expanded) prepends ▾. - layout.css: .ft-chevron (flex:none + 14px + fill currentColor) +
flex-shrink: 0on .ft-row — correct root cause: .result-files flex column + overflow-y:auto, .ft-row shrink:1 + overflow:hidden collapses min-height to 0, tree compresses to container height so scrollHeight==clientHeight. Fix restores the intended scroll. - renderer.smoke.test.js: +1 test covers root ▾ / collapsed ▸ / click→▾ / files no-chevron.
- Agent.md GUI count 237→238 synced.
Checks: test + test-windows PASS. mergeable: CLEAN. Approving.
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle (2/3)
Full-diff review of the feature code (f37ecc9, unchanged in cf32eb6):
- file-tree.js:
chevronRight/chevronDownicons +chevronFor(); dir rows prepend chevron in.ft-head;toggleDirsyncs both the dir icon and chevron; root row defaults to ▾ (chevronDown) with click binding (#733) preserved. - layout.css:
.ft-row { flex-shrink: 0 }— fixes the scroll-compression root cause (flex column item with overflow:hidden collapses to container height → scrollHeight==clientHeight);.ft-chevronsizing/margin. - renderer.smoke.test.js: P3.5 test covers root ▾, collapsed dir ▸, click → ▾, and file rows have no chevron (positive+negative).
cf32eb6 = doc reconciliation: Agent.md GUI count bumped 238→239 (117 renderer smoke) since #768 merged first (both PRs add 1 renderer smoke test); merged master will truly have 239. Verified: pytest 801, GUI 238/238 on branch, node --check, import + CLI OK, CI test + test-windows PASS.
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle (3/3)
Head advanced f37ecc9 → 954dc9a (author doc-sync cf32eb6: Agent.md 238→239 / renderer smoke 116→117 to account for #768's merged regression test — verified the count math: master's #768 test + this PR's +1 chevron test = 239/117, correct; I then merged master in to clear the Agent.md same-line conflict — resolution kept 239/117, verified the merged-tree diff vs master is exactly the intended 4-file delta: Agent.md + layout.css + file-tree.js + renderer.smoke.test.js).
Fresh re-verification of the resolved head (code content unchanged since the 2/3 review):
- file-tree.js: chevronRight/chevronDown ICONs, iconSvg(d, cls) backward-compatible, chevronFor() reads the same
expandedMap as dirOpen/dirClosed (desync-proof), toggleDir syncs both icons, root row ▾ default, file rows no chevron. - layout.css: .ft-chevron (flex:none 14px) + .ft-row flex-shrink:0 — root cause confirmed against the cascade (.result-files flex column + overflow-y:auto, .ft-row shrink:1+overflow:hidden → min-height:auto 0 → tree compressed → scrollHeight==clientHeight).
- renderer.smoke.test.js: +1 chevron test (root ▾ / collapsed ▸ / click → ▾ / files no-chevron).
- Agent.md 239 sync (both doc sites).
CI run 31704433911 test + test-windows PASS on 954dc9a. mergeable: MERGEABLE/CLEAN. Approving.
Summary
Implements host rant 2026-08-13T20:58:57 — two file-browser issues:
Changes
file-tree.js: addchevronRight/chevronDowntoICON;iconSvg(d, cls)gains an optional class param;chevronFor(entry)maps expanded state → chevron path; directory rows (and the root row) prepend a.ft-chevronbefore the folder icon;toggleDirsyncs the chevroninnerHTMLon expand/collapse. File rows get no chevron.layout.css: addflex-shrink: 0to.ft-row—.ft-rowis a flex item of.result-files(flex column) withoverflow:hidden, so itsmin-height:autoresolves to 0 and the whole tree compresses to container height (scrollHeight == clientHeight, never scrolling).flex-shrink: 0prevents the compression. Add.ft-chevronstyle.renderer.smoke.test.js: add P3.5 test asserting root chevron = ▾ (M3 6l5 5 5-5z), collapsed dir chevron = ▸ (M6 3l5 5-5 5z), expands to ▾ on click, and file rows have no chevron.Agent.md: GUI test count 237 → 238 (renderer smoke 115 → 116).Verification
node --checkon touched JS: OKcd emrg/gui && npm test: 238/238 passuv run pytest tests/ -q: 801 passeduv run python -c "from emrg.client.app import run_client"+uv run python -m emrg --help: OK