fix(ui): persistent session banner for boot + session context#359
Conversation
… scroll away Move BootContextPill and ContextBlock out of the scrollable chat-messages container into a new SessionBanner component rendered between the header and message area. The banner is always visible, defaults to a compact one-liner, and expands with its own scroll (max 40vh) for full details. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Centaur ReviewFound 4 issue(s) (1 critical) (2 warning).
|
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
dimakis
left a comment
There was a problem hiding this comment.
Centaur Review
Found 5 issue(s) (2 warning).
frontend/src/components/BootContextPill.tsx
Clean structural refactor moving session context out of the scroll container. Main issues: two orphaned component files (BootContextPill, ContextBlock) should be deleted, nested button is invalid HTML, and no test coverage for the new component.
- 🟡 style: BootContextPill.tsx is now dead code — no remaining imports anywhere in the codebase after ChatArea stopped using it. Should be deleted in this PR along with its CSS (.boot-context-pill styles in global.css).
[fixable]
frontend/src/components/ContextBlock.tsx
Clean structural refactor moving session context out of the scroll container. Main issues: two orphaned component files (BootContextPill, ContextBlock) should be deleted, nested button is invalid HTML, and no test coverage for the new component.
- 🟡 style: ContextBlock.tsx is now dead code — no remaining imports. Its CSS (.context-block styles, kept as '— legacy' in global.css) and the file itself should be removed in this PR.
[fixable]
frontend/src/components/SessionBanner.tsx
Clean structural refactor moving session context out of the scroll container. Main issues: two orphaned component files (BootContextPill, ContextBlock) should be deleted, nested button is invalid HTML, and no test coverage for the new component.
- 🔵 missing_tests: New 217-line component with expand/collapse, modal, and conditional rendering has no test file. A basic test covering: renders null when both props are null, renders banner header when bootContext provided, renders session context summary when sessionContext provided, and expand toggle would prevent regressions.
[fixable] - 🔵 bugs (L143): The nested button (.session-banner-view-full '⧉') inside the outer button (.session-banner-boot-toggle) is invalid HTML (buttons cannot nest). While e.stopPropagation prevents functional issues in most browsers, screen readers and HTML validators will flag this. Consider using a span with role='button' and tabIndex/onKeyDown, or restructure so the outer element is a div with onClick.
[fixable] - 🔵 style (L48): The component manages 4 independent boolean states (expanded, showBootDetails, showTrimmed, showModal). These never reset when props change — if a user expands details for one session, switches to another session with different context, the expand states persist from the prior session. Consider adding a useEffect that resets expand states when bootContext/sessionContext identity changes.
[fixable]
| {showBootDetails && ( | ||
| <div className="session-banner-boot-content"> | ||
| <div className="session-banner-sub-label">Sources</div> | ||
| {bootContext.sources.map((src, idx) => ( |
There was a problem hiding this comment.
🔵 bugs: The nested button (.session-banner-view-full '⧉') inside the outer button (.session-banner-boot-toggle) is invalid HTML (buttons cannot nest). While e.stopPropagation prevents functional issues in most browsers, screen readers and HTML validators will flag this. Consider using a span with role='button' and tabIndex/onKeyDown, or restructure so the outer element is a div with onClick. [fixable]
| return first.length > 80 ? first.slice(0, 77) + '...' : first; | ||
| } | ||
|
|
||
| export function SessionBanner({ bootContext, sessionContext }: Props) { |
There was a problem hiding this comment.
🔵 style: The component manages 4 independent boolean states (expanded, showBootDetails, showTrimmed, showModal). These never reset when props change — if a user expands details for one session, switches to another session with different context, the expand states persist from the prior session. Consider adding a useEffect that resets expand states when bootContext/sessionContext identity changes. [fixable]
- Delete orphaned BootContextPill.tsx and ContextBlock.tsx - Remove all legacy .context-block and .boot-context-pill CSS - Fix nested button (invalid HTML) — use div[role=button] for boot toggle - Reset expand states on session switch via useEffect - Add SessionBanner test suite (5 tests) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
/centaur-review |
dimakis
left a comment
There was a problem hiding this comment.
Centaur Review
Found 3 issue(s) (1 warning).
frontend/src/components/SessionBanner.tsx
Clean consolidation of two components into one persistent banner. No regressions in callers or tests. Minor a11y nit with nested interactive control inside role="button" and a weak context-change key.
- 🟡 bugs (L141): A
<button>("view full markdown") is nested inside a<div role="button">(boot-toggle). While this fixes the previous invalid<button>inside<button>, the ARIA spec still flags interactive controls insiderole="button"as problematic for assistive technology. Consider moving the ⧉ button outside the toggle div, e.g. as a sibling in a wrapper flex row.[fixable] - 🔵 unsafe_assumptions (L65):
contextKeyusestokenCount + '|' + sessionContextto detect session switches. If two sessions share the same tokenCount and sessionContext, the expand states won't reset. A more robust key could includesourceCountor the first source path, e.g.bootContext?.sources[0]?.path.[fixable]
frontend/src/components/__tests__/SessionBanner.test.tsx
Clean consolidation of two components into one persistent banner. No regressions in callers or tests. Minor a11y nit with nested interactive control inside role="button" and a weak context-change key.
- 🔵 missing_tests: Tests cover collapsed rendering and first-level expand, but don't cover: boot context details expansion (showBootDetails toggle), trimmed sections toggle, the full-markdown modal open/close, or the Escape key handler. These are interactive paths ported from the deleted BootContextPill and could regress silently.
[fixable]
| Boot Context ({isContexgin ? 'ContexGin' : 'Fallback'}) | ||
| </span> | ||
| {bootContext.fullMarkdown && ( | ||
| <button |
There was a problem hiding this comment.
🟡 bugs: A <button> ("view full markdown") is nested inside a <div role="button"> (boot-toggle). While this fixes the previous invalid <button> inside <button>, the ARIA spec still flags interactive controls inside role="button" as problematic for assistive technology. Consider moving the ⧉ button outside the toggle div, e.g. as a sibling in a wrapper flex row. [fixable]
| }, [showModal]); | ||
|
|
||
| // Reset expand states when context identity changes (e.g. session switch) | ||
| const contextKey = (bootContext?.tokenCount ?? '') + '|' + (sessionContext ?? ''); |
There was a problem hiding this comment.
🔵 unsafe_assumptions: contextKey uses tokenCount + '|' + sessionContext to detect session switches. If two sessions share the same tokenCount and sessionContext, the expand states won't reset. A more robust key could include sourceCount or the first source path, e.g. bootContext?.sources[0]?.path. [fixable]
…ests - Move ⧉ button outside role="button" div as sibling in flex row - Strengthen contextKey with sourceCount + first source path - Add tests for boot details toggle, modal open/close, Escape key Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Summary
Fixes the mobile UX issue where boot/session context would clip, become unscrollable, and disappear entirely as chat messages accumulated.
Test plan
🤖 Generated with Claude Code