ui: split-pane detail view + responsive top-bar rail#44
Conversation
Detail panels (episodes, observations, reflections, semantic) now render in a right-side pane via a 2px ink rule split layout, instead of below the list where they often scrolled off-screen. CSS :has() toggles the layout based on whether the drawer container has content, so no extra JS state is needed. At <=1080px viewport the left rail flips to a horizontal top bar and the split collapses to a drill-in pattern: selecting a row replaces the list with the detail full-width, and a back-link returns to the list. At <=720px the top bar drops labels, leaving the numbered chips. Diagnostics is unchanged (two-panel layout doesn't fit the split shape). All 116 UI tests pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
🔴 Claude BugBot Analysis
Found 2 potential bugs in this PR.
medium: 1 | low: 1
The main defect is a state-desync bug: the JS click handler stores selection solely as a CSS class that HTMX silently erases on every 30-second list poll, leaving the detail pane showing content for an apparently unselected row. A secondary low-severity risk is that the hardcoded inline onclick handlers in all four drawer fragments will throw a TypeError if their target container element is missing from the DOM.
|
|
||
| // Row selection: mark the clicked row in the list pane and clear it | ||
| // when the drawer is closed via close-drawer or back-link. | ||
| document.addEventListener("click", function (e) { |
There was a problem hiding this comment.
🟡 MEDIUM: Selection highlight lost on HTMX polling refresh
The document-level click handler stores row selection state exclusively as a CSS class (selected) on a DOM element. Every 30 seconds, HTMX polls with hx-trigger="load, every 30s" and replaces the innerHTML of the list panel (e.g., #timeline, #observation-panel) with fresh server-rendered HTML. The new HTML contains no selected class, so the highlight is silently erased. Meanwhile, the detail pane continues to display the previously selected item's drawer content. This leaves the UI in an inconsistent state: the detail pane shows content for an item that appears unselected in the list. Fix: add an htmx:afterSwap event listener that reads the last-selected item's identity from a variable (or data- attribute) and re-applies the selected class to the matching row after each poll swap.
Additional Locations
better_memory/ui/templates/episodes.html:15— List panel polling trigger that replaces rows and erases the selected classbetter_memory/ui/templates/observations.html:12— Observation panel polling trigger with same issuebetter_memory/ui/templates/reflections.html:12— Reflection panel polling trigger with same issuebetter_memory/ui/templates/semantic.html:12— Semantic panel polling trigger with same issue
| <div class="episode-drawer" id="episode-drawer-{{ detail.episode.id }}"> | ||
| <button class="back-link" | ||
| type="button" | ||
| onclick="document.getElementById('drawer').innerHTML = '';"> |
There was a problem hiding this comment.
🔵 LOW: Null dereference in back-link onclick if container element is absent
The inline onclick handler uses document.getElementById('drawer').innerHTML = '' without a null guard. If the fragment is ever loaded in a context where #drawer does not exist in the DOM (e.g., a future route, a test harness, or a partial embed), getElementById returns null and the .innerHTML assignment throws a TypeError, breaking the UI. Fix: guard with optional chaining — document.getElementById('drawer')?.innerHTML = '' — or extract to a named function that performs the null check.
Additional Locations
better_memory/ui/templates/fragments/observation_drawer.html:4— Same pattern targeting#observation-drawerbetter_memory/ui/templates/fragments/reflection_drawer.html:4— Same pattern targeting#reflection-drawerbetter_memory/ui/templates/fragments/semantic_drawer.html:4— Same pattern targeting#semantic-drawer
Two BugBot findings on PR #44: 1. medium — list panel polls every 30s and innerHTML-swaps the timeline, which wipes the .selected class. Drawer stays visible so the detail pane shows content for an apparently unselected row. 2. low — inline `getElementById(...).innerHTML = ''` handlers in the back-link and close-drawer buttons throw TypeError if the container element is missing. Fix: - Each row now carries `data-row-id="{{ row.id }}"`. The click handler stores the selected id in a per-list-pane WeakMap, and an `htmx:afterSettle` listener re-applies `.selected` to the matching row whenever its panel swaps. - Back-link, close-drawer and the semantic-drawer cancel button no longer carry inline `onclick`. The base.html delegated handler walks up to `.detail-pane` and clears its container divs by scope. Missing-element case is handled by the `if (pane)` guard. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Both findings addressed in 25f1628: 1. State desync (medium). Each row now carries 2. Inline onclick TypeError (low). Removed inline |
There was a problem hiding this comment.
🟢 Claude BugBot Analysis
The PR introduces a split-pane layout with HTMX-aware row selection persistence. No new bugs were found in the added or modified lines. Both previously reported bugs have been resolved: the selection highlight is now re-applied after each HTMX poll swap via a htmx:afterSettle listener, and all inline onclick null-dereference patterns on close/cancel buttons have been replaced with safe event delegation in base.html.
No bugs were detected in this PR.
1 participant
Summary
How it works
Scope
Test plan
🤖 Generated with Claude Code