Fix document inspector UI issues and improve accessibility#644
Conversation
- Fixed copy button visibility when summaries overflow - Added proper flex constraints and text truncation - Enhanced metadata display in footer with relevance scores - Improved accessibility with ARIA labels and semantic HTML - Added better loading states and error handling - Enhanced visual feedback with motion effects
|
Caution Review failedThe pull request is closed. WalkthroughReworks ContentViewer header/body/footer for proper truncation and metadata display; InspectorSidebar gains semantic aside, ARIA improvements, better loading/empty states, and overflow/truncation fixes; KnowledgeInspector memoizes handlers (copy, selection, mode changes) and hardens clipboard logic. No public API changes. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as User
participant IS as InspectorSidebar
participant KI as KnowledgeInspector
participant CV as ContentViewer
participant CB as Clipboard API
U->>IS: Search / Select item
IS->>KI: onItemSelect(item, viewMode)
KI->>KI: set selectedItem (memoized)
KI-->>CV: selectedItem props update
CV-->>U: Render content, metadata, footer
U->>CV: Click Copy
CV->>KI: onCopy(contentId, text)
KI->>CB: navigator.clipboard.writeText(text)
alt Copy success
KI->>KI: set copiedId, clear after 2s
KI-->>CV: copied state updates
else Copy failure
KI->>KI: log error
end
U->>KI: Change view mode
KI->>KI: set viewMode, clear selectedItem, reset search
KI-->>IS: props update (viewMode, query)
IS-->>U: Render list / loading / empty state
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
✨ Finishing touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
archon-ui-main/src/features/knowledge/inspector/components/ContentViewer.tsx (2)
84-103: Add an explicit accessible label to the Copy button.Improves SR clarity when focus lands on the control.
- <Button + <Button size="sm" variant="ghost" onClick={() => onCopy(selectedItem.content, selectedItem.id)} - className="text-gray-400 hover:text-white flex-shrink-0" + className="text-gray-400 hover:text-white flex-shrink-0" + aria-label={`Copy ${selectedItem.type} content`} >
109-121: Optional: expose language for future syntax highlighting.If a highlighter is introduced later, pre-adding a class or data attr avoids refactors.
Example:
- <pre className="bg-black/30 border border-white/10 rounded-lg p-4 overflow-x-auto scrollbar-thin scrollbar-thumb-white/10 scrollbar-track-transparent"> - <code className="text-sm text-gray-300 font-mono"> + <pre className="bg-black/30 border border-white/10 rounded-lg p-4 overflow-x-auto scrollbar-thin scrollbar-thumb-white/10 scrollbar-track-transparent"> + <code + className={cn( + "text-sm text-gray-300 font-mono", + selectedItem.metadata && "language" in selectedItem.metadata && selectedItem.metadata.language + ? `language-${selectedItem.metadata.language.toLowerCase()}` + : undefined, + )} + data-language={ + selectedItem.metadata && "language" in selectedItem.metadata ? selectedItem.metadata.language : undefined + } + > {selectedItem.content || "// No code content available"} </code>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
archon-ui-main/src/features/knowledge/inspector/components/ContentViewer.tsx(3 hunks)archon-ui-main/src/features/knowledge/inspector/components/InspectorSidebar.tsx(2 hunks)archon-ui-main/src/features/knowledge/inspector/components/KnowledgeInspector.tsx(2 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
archon-ui-main/src/features/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
archon-ui-main/src/features/**/*.{ts,tsx}: Implement optimistic updates with rollback in TanStack Query mutations
Use Biome formatting and lint rules in /src/features with 120-character lines, double quotes, and trailing commas
Implement HTTP polling patterns: 1–2s for active ops, 5–10s for background, smart pausing on tab inactivity, use ETag caching where applicable
archon-ui-main/src/features/**/*.{ts,tsx}: Follow TanStack Query patterns: query-keys factory, smart polling via useSmartPolling, optimistic updates with rollback
Biome formatting in features: 120-character lines, double quotes, trailing commas
Files:
archon-ui-main/src/features/knowledge/inspector/components/KnowledgeInspector.tsxarchon-ui-main/src/features/knowledge/inspector/components/ContentViewer.tsxarchon-ui-main/src/features/knowledge/inspector/components/InspectorSidebar.tsx
archon-ui-main/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use state naming conventions: is[Action]ing for loading, [resource]Error for errors, selected[Resource] for selections
archon-ui-main/src/**/*.{ts,tsx}: Use TanStack Query for all data fetching; avoid prop drilling
TypeScript: strict mode with no implicit any in frontend code
State naming: is[Action]ing for loading flags, [resource]Error for errors, selected[Resource] for current selection
Use HTTP polling with ETag caching; do not introduce WebSocket-based updates in the frontend
Files:
archon-ui-main/src/features/knowledge/inspector/components/KnowledgeInspector.tsxarchon-ui-main/src/features/knowledge/inspector/components/ContentViewer.tsxarchon-ui-main/src/features/knowledge/inspector/components/InspectorSidebar.tsx
🧬 Code graph analysis (3)
archon-ui-main/src/features/knowledge/inspector/components/KnowledgeInspector.tsx (1)
archon-ui-main/src/features/knowledge/types/knowledge.ts (2)
DocumentChunk(73-85)CodeExample(49-62)
archon-ui-main/src/features/knowledge/inspector/components/ContentViewer.tsx (1)
archon-ui-main/src/features/ui/primitives/button.tsx (1)
Button(11-130)
archon-ui-main/src/features/knowledge/inspector/components/InspectorSidebar.tsx (3)
archon-ui-main/src/features/ui/primitives/input.tsx (1)
Input(8-29)archon-ui-main/src/features/ui/primitives/styles.ts (1)
cn(139-141)archon-ui-main/src/features/knowledge/types/knowledge.ts (1)
CodeExample(49-62)
🔇 Additional comments (8)
archon-ui-main/src/features/knowledge/inspector/components/KnowledgeInspector.tsx (2)
6-6: Importing useCallback for stable handlers — good call.This aligns with your re-render and performance goals.
127-131: Stable view-mode change handler — LGTM.Resets selection and search predictably; dependency [] is correct.
archon-ui-main/src/features/knowledge/inspector/components/ContentViewer.tsx (2)
30-41: Header truncation and non-shrinking icon — solid.The min-w-0/flex-1 pattern fixes overflow regressions.
124-147: Footer relevance/view-source UX — looks good.Semantic link, percentage formatting, and stable footer wrapper are on point.
archon-ui-main/src/features/knowledge/inspector/components/InspectorSidebar.tsx (4)
with aria-label — nice accessibility upgrade.
64-71: UsingIcon marked aria-hidden and non-interactive is correct.
85-94: Loading/empty states are accessible and informative.aria-live="polite" and contextual hints on empty search are well done.
125-138: Title and description truncation with tooltips — solid.Good min-w-0 usage and language badge non-shrinking.
153-173: Load-more control UX/a11y — LGTM.Clear labeling, spinner hidden from SR when not needed, and full-width target.
| whileTap={{ scale: 0.98 }} | ||
| onClick={() => onItemSelect(item)} | ||
| className={cn( | ||
| "w-full text-left p-3 rounded-lg mb-1 transition-colors", | ||
| "hover:bg-white/5", | ||
| selectedItemId === item.id ? "bg-cyan-500/10 border border-cyan-500/30" : "border border-transparent", | ||
| "w-full text-left p-3 rounded-lg mb-1 transition-all", | ||
| "hover:bg-white/5 focus:outline-none focus:ring-2 focus:ring-cyan-500/50", | ||
| selectedItemId === item.id | ||
| ? "bg-cyan-500/10 border border-cyan-500/30 ring-1 ring-cyan-500/20" | ||
| : "border border-transparent", | ||
| )} | ||
| aria-selected={selectedItemId === item.id} | ||
| aria-label={`${getItemTitle(item)}. ${getItemDescription(item)}`} | ||
| > |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Button semantics conflict with aria-selected.
aria-selected is intended for options in selectable containers (e.g., listbox). For buttons, either use aria-pressed or switch items to role="option".
Minimal change to keep current structure:
- <motion.button
+ <motion.button
type="button"
key={item.id}
whileHover={{ x: 2 }}
whileTap={{ scale: 0.98 }}
onClick={() => onItemSelect(item)}
className={cn(
"w-full text-left p-3 rounded-lg mb-1 transition-all",
"hover:bg-white/5 focus:outline-none focus:ring-2 focus:ring-cyan-500/50",
selectedItemId === item.id
? "bg-cyan-500/10 border border-cyan-500/30 ring-1 ring-cyan-500/20"
: "border border-transparent",
)}
- aria-selected={selectedItemId === item.id}
+ role="option"
+ aria-selected={selectedItemId === item.id}
aria-label={`${getItemTitle(item)}. ${getItemDescription(item)}`}
>If you prefer staying with buttons, alternatively use:
- aria-selected={selectedItemId === item.id}
+ aria-pressed={selectedItemId === item.id}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| whileTap={{ scale: 0.98 }} | |
| onClick={() => onItemSelect(item)} | |
| className={cn( | |
| "w-full text-left p-3 rounded-lg mb-1 transition-colors", | |
| "hover:bg-white/5", | |
| selectedItemId === item.id ? "bg-cyan-500/10 border border-cyan-500/30" : "border border-transparent", | |
| "w-full text-left p-3 rounded-lg mb-1 transition-all", | |
| "hover:bg-white/5 focus:outline-none focus:ring-2 focus:ring-cyan-500/50", | |
| selectedItemId === item.id | |
| ? "bg-cyan-500/10 border border-cyan-500/30 ring-1 ring-cyan-500/20" | |
| : "border border-transparent", | |
| )} | |
| aria-selected={selectedItemId === item.id} | |
| aria-label={`${getItemTitle(item)}. ${getItemDescription(item)}`} | |
| > | |
| <motion.button | |
| type="button" | |
| key={item.id} | |
| whileHover={{ x: 2 }} | |
| whileTap={{ scale: 0.98 }} | |
| onClick={() => onItemSelect(item)} | |
| className={cn( | |
| "w-full text-left p-3 rounded-lg mb-1 transition-all", | |
| "hover:bg-white/5 focus:outline-none focus:ring-2 focus:ring-cyan-500/50", | |
| selectedItemId === item.id | |
| ? "bg-cyan-500/10 border border-cyan-500/30 ring-1 ring-cyan-500/20" | |
| : "border border-transparent", | |
| )} | |
| role="option" | |
| aria-selected={selectedItemId === item.id} | |
| aria-label={`${getItemTitle(item)}. ${getItemDescription(item)}`} | |
| > |
🤖 Prompt for AI Agents
In
archon-ui-main/src/features/knowledge/inspector/components/InspectorSidebar.tsx
around lines 101 to 112, the element is a button but uses aria-selected (meant
for selectable listbox/options), so either (A) keep the button semantics and
replace aria-selected with aria-pressed and ensure keyboard/toggle behavior
reflects a pressed state, or (B) treat the items as options by changing role to
"option" on the element, ensure the parent has the appropriate listbox/combobox
role, update keyboard handling to option semantics, and remove aria-pressed;
implement the chosen approach consistently across selection styling and event
handlers.
| const handleCopy = useCallback(async (text: string, id: string) => { | ||
| try { | ||
| await navigator.clipboard.writeText(text); | ||
| setCopiedId(id); | ||
| setTimeout(() => setCopiedId(null), 2000); | ||
| } catch (error) { | ||
| console.error("Failed to copy to clipboard:", error); | ||
| } | ||
| }; | ||
| }, []); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Copy success timeout can clear a newer “Copied!” state (race).
If users copy A then B within 2s, A’s timer nulls B’s state. Guard the clear by id and prefer clearing any prior timer.
Apply within this block:
- setTimeout(() => setCopiedId(null), 2000);
+ setTimeout(() => setCopiedId((v) => (v === id ? null : v)), 2000);Additionally, consider centralizing timeout management:
// Add near state declarations:
const copyTimeoutRef = useRef<number | null>(null);
// In handleCopy:
if (copyTimeoutRef.current) clearTimeout(copyTimeoutRef.current);
copyTimeoutRef.current = window.setTimeout(() => {
setCopiedId((v) => (v === id ? null : v));
}, 2000);
// Cleanup on unmount:
useEffect(() => () => copyTimeoutRef.current && clearTimeout(copyTimeoutRef.current), []);🤖 Prompt for AI Agents
In
archon-ui-main/src/features/knowledge/inspector/components/KnowledgeInspector.tsx
around lines 82 to 90, the current copy timeout can race and clear a newer
"Copied!" state; change handleCopy to use a ref-based timeout: add a
copyTimeoutRef via useRef<number | null>(null) near the state declarations,
clear any existing timeout at the start of handleCopy, set copiedId and then set
copyTimeoutRef to window.setTimeout that only clears the state if the current
copiedId matches the id (i.e., setCopiedId(v => v === id ? null : v)), and add a
useEffect cleanup to clear copyTimeoutRef on unmount. Ensure typings use number
for the timeout id and that clearTimeout is called on the stored id before
reassigning.
| const handleItemSelect = useCallback( | ||
| (item: DocumentChunk | CodeExample) => { | ||
| if (viewMode === "documents") { | ||
| const doc = item as DocumentChunk; | ||
| setSelectedItem({ | ||
| type: "document", | ||
| id: doc.id || "", | ||
| content: doc.content || "", | ||
| metadata: { | ||
| title: doc.metadata?.title, | ||
| section: doc.metadata?.section, | ||
| relevance_score: doc.metadata?.relevance_score, | ||
| url: doc.metadata?.url, | ||
| tags: doc.metadata?.tags, | ||
| }, | ||
| }); | ||
| } else { | ||
| const code = item as CodeExample; | ||
| setSelectedItem({ | ||
| type: "code", | ||
| id: String(code.id), | ||
| content: code.content || code.code || "", | ||
| metadata: { | ||
| language: code.language, | ||
| file_path: code.file_path, | ||
| summary: code.summary, | ||
| relevance_score: code.metadata?.relevance_score, | ||
| title: code.title || code.example_name, | ||
| }, | ||
| }); | ||
| } | ||
| }, | ||
| [viewMode], | ||
| ); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Document selection metadata misses top-level fallbacks (title/section/url).
Manual selection uses only metadata fields, while auto-select uses top-level fields first. This causes inconsistent headers and missing URLs.
- metadata: {
- title: doc.metadata?.title,
- section: doc.metadata?.section,
- relevance_score: doc.metadata?.relevance_score,
- url: doc.metadata?.url,
- tags: doc.metadata?.tags,
- },
+ metadata: {
+ title: doc.title || doc.metadata?.title,
+ section: doc.section || doc.metadata?.section,
+ relevance_score: doc.metadata?.relevance_score,
+ url: doc.url || doc.metadata?.url,
+ tags: doc.metadata?.tags,
+ },📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const handleItemSelect = useCallback( | |
| (item: DocumentChunk | CodeExample) => { | |
| if (viewMode === "documents") { | |
| const doc = item as DocumentChunk; | |
| setSelectedItem({ | |
| type: "document", | |
| id: doc.id || "", | |
| content: doc.content || "", | |
| metadata: { | |
| title: doc.metadata?.title, | |
| section: doc.metadata?.section, | |
| relevance_score: doc.metadata?.relevance_score, | |
| url: doc.metadata?.url, | |
| tags: doc.metadata?.tags, | |
| }, | |
| }); | |
| } else { | |
| const code = item as CodeExample; | |
| setSelectedItem({ | |
| type: "code", | |
| id: String(code.id), | |
| content: code.content || code.code || "", | |
| metadata: { | |
| language: code.language, | |
| file_path: code.file_path, | |
| summary: code.summary, | |
| relevance_score: code.metadata?.relevance_score, | |
| title: code.title || code.example_name, | |
| }, | |
| }); | |
| } | |
| }, | |
| [viewMode], | |
| ); | |
| const handleItemSelect = useCallback( | |
| (item: DocumentChunk | CodeExample) => { | |
| if (viewMode === "documents") { | |
| const doc = item as DocumentChunk; | |
| setSelectedItem({ | |
| type: "document", | |
| id: doc.id || "", | |
| content: doc.content || "", | |
| metadata: { | |
| title: doc.title || doc.metadata?.title, | |
| section: doc.section || doc.metadata?.section, | |
| relevance_score: doc.metadata?.relevance_score, | |
| url: doc.url || doc.metadata?.url, | |
| tags: doc.metadata?.tags, | |
| }, | |
| }); | |
| } else { | |
| const code = item as CodeExample; | |
| setSelectedItem({ | |
| type: "code", | |
| id: String(code.id), | |
| content: code.content || code.code || "", | |
| metadata: { | |
| language: code.language, | |
| file_path: code.file_path, | |
| summary: code.summary, | |
| relevance_score: code.metadata?.relevance_score, | |
| title: code.title || code.example_name, | |
| }, | |
| }); | |
| } | |
| }, | |
| [viewMode], | |
| ); |
🤖 Prompt for AI Agents
In
archon-ui-main/src/features/knowledge/inspector/components/KnowledgeInspector.tsx
around lines 92 to 125, the document-selection branch only pulls
title/section/url from metadata causing missing headers/URLs; update the
document case to fallback to top-level fields when metadata is absent (e.g.,
title: doc.metadata?.title || doc.title, section: doc.metadata?.section ||
doc.section, url: doc.metadata?.url || doc.url) and similarly ensure
tags/relevance_score also fall back to top-level equivalents if present so auto-
and manual-selection produce consistent metadata.
- Fixed copy timeout race condition using functional setState - Added role="option" for proper aria-selected semantics
- Align manual selection with auto-selection pattern - Use top-level fields as primary source with metadata as fallback - Ensures consistent display of titles, sections, and URLs
) * Fix document inspector UI issues and improve accessibility - Fixed copy button visibility when summaries overflow - Added proper flex constraints and text truncation - Enhanced metadata display in footer with relevance scores - Improved accessibility with ARIA labels and semantic HTML - Added better loading states and error handling - Enhanced visual feedback with motion effects * Fix CodeRabbit review issues - Fixed copy timeout race condition using functional setState - Added role="option" for proper aria-selected semantics * Fix document metadata inconsistency in manual selection - Align manual selection with auto-selection pattern - Use top-level fields as primary source with metadata as fallback - Ensures consistent display of titles, sections, and URLs
Fix all Windows test failures across 6 packages by addressing three root causes: path separator mismatches, symlink permission errors, and SQLite test isolation. **Production bug fixes (2 files):** - `packages/paths/src/archon-paths.ts`: Fix `resolveProjectRootFromCwd` splitting on '/' only — now splits on both '/' and '\' via regex - `packages/git/src/worktree.ts`: Same fix in `getWorktreeBase` and `isProjectScopedWorktreeBase` **Test fixes (8 files):** - `packages/paths/src/archon-paths.test.ts`: Use `path.join()` for Docker and ARCHON_HOME tests; skip symlink tests on Windows via `test.skipIf` - `packages/git/src/git.test.ts`: Use `path.join()` for ARCHON_HOME path - `packages/core/src/utils/worktree-sync.test.ts`: Add `normPath()` helper, normalize all path comparisons in mocks - `packages/core/src/utils/defaults-copy.test.ts`: Add `normPath()` helper, normalize `includes()` checks and mkdir assertions - `packages/isolation/src/providers/worktree.test.ts`: Normalize orphan directory path assertion for Windows backslashes - `packages/isolation/src/worktree-copy.test.ts`: Use `path.join()` for copy/mkdir path expectations - `packages/workflows/src/loader.test.ts`: Skip symlink test on Windows - `packages/core/src/db/adapters/sqlite.test.ts`: Unique DB path per test Closes #644 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Fix all Windows test failures across 6 packages by addressing three root causes: path separator mismatches, symlink permission errors, and SQLite test isolation. **Production bug fixes (2 files):** - `packages/paths/src/archon-paths.ts`: Fix `resolveProjectRootFromCwd` splitting on '/' only — now splits on both '/' and '\' via regex - `packages/git/src/worktree.ts`: Same fix in `getWorktreeBase` and `isProjectScopedWorktreeBase` **Test fixes (8 files):** - `packages/paths/src/archon-paths.test.ts`: Use `path.join()` for Docker and ARCHON_HOME tests; skip symlink tests on Windows via `test.skipIf` - `packages/git/src/git.test.ts`: Use `path.join()` for ARCHON_HOME path - `packages/core/src/utils/worktree-sync.test.ts`: Add `normPath()` helper, normalize all path comparisons in mocks - `packages/core/src/utils/defaults-copy.test.ts`: Add `normPath()` helper, normalize `includes()` checks and mkdir assertions - `packages/isolation/src/providers/worktree.test.ts`: Normalize orphan directory path assertion for Windows backslashes - `packages/isolation/src/worktree-copy.test.ts`: Use `path.join()` for copy/mkdir path expectations - `packages/workflows/src/loader.test.ts`: Skip symlink test on Windows - `packages/core/src/db/adapters/sqlite.test.ts`: Unique DB path per test Closes coleam00#644 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Fix all Windows test failures across 6 packages by addressing three root causes: path separator mismatches, symlink permission errors, and SQLite test isolation. **Production bug fixes (2 files):** - `packages/paths/src/archon-paths.ts`: Fix `resolveProjectRootFromCwd` splitting on '/' only — now splits on both '/' and '\' via regex - `packages/git/src/worktree.ts`: Same fix in `getWorktreeBase` and `isProjectScopedWorktreeBase` **Test fixes (8 files):** - `packages/paths/src/archon-paths.test.ts`: Use `path.join()` for Docker and ARCHON_HOME tests; skip symlink tests on Windows via `test.skipIf` - `packages/git/src/git.test.ts`: Use `path.join()` for ARCHON_HOME path - `packages/core/src/utils/worktree-sync.test.ts`: Add `normPath()` helper, normalize all path comparisons in mocks - `packages/core/src/utils/defaults-copy.test.ts`: Add `normPath()` helper, normalize `includes()` checks and mkdir assertions - `packages/isolation/src/providers/worktree.test.ts`: Normalize orphan directory path assertion for Windows backslashes - `packages/isolation/src/worktree-copy.test.ts`: Use `path.join()` for copy/mkdir path expectations - `packages/workflows/src/loader.test.ts`: Skip symlink test on Windows - `packages/core/src/db/adapters/sqlite.test.ts`: Unique DB path per test Closes coleam00#644 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Summary
Fixes UI issues in the document inspector where copy buttons were being pushed off-screen by long summaries, and improves overall accessibility and user experience.
Problem
Solution
Layout Fixes
min-w-0,flex-1,flex-shrink-0) to prevent overflowtruncateandline-clamp-2classesAccessibility Improvements
UI Enhancements
Testing
Screenshots
Before: Copy button was pushed off-screen with long summaries
After: Copy button stays visible, text properly truncates
Related Issues
Addresses user-reported issues with document inspector UI
Summary by CodeRabbit