-
Notifications
You must be signed in to change notification settings - Fork 11
🤖 Review panel read state tracking and visual improvements #323
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…iction - Content-based hunk IDs: Hash file path + diff content for rebase stability - New useReviewState hook with simplified API (isRead, markAsRead, toggleRead) - LRU eviction: Automatically keeps newest 1024 read states per workspace - Visual indicators: Green checkmark and background tint for read hunks - Auto-collapse: Read hunks collapse by default - Filter toggle: Show/hide read hunks with per-workspace persistence - Keyboard shortcut: 'm' key toggles read state of selected hunk - Stats display: Shows 'X read / Y total' in review controls Generated with `cmux`
When navigating hunks with j/k keys, automatically scroll the viewport to keep the selected hunk visible using smooth scrolling behavior. - Added data-hunk-id attribute to HunkViewer container - Added useEffect to scroll selected hunk into view on change - Uses scrollIntoView with 'nearest' block positioning Generated with `cmux`
- Simplified useReviewState.markAsRead to accept string | string[] - Added onMarkFileAsRead handler to mark all hunks in a file - Added getFileReadStatus to show read/total counts per file - Visual indicator: Show ✓ button when file has unread hunks - Shows filled ✓ when all hunks in file are read - Button tooltip: 'Mark all hunks as read' Generated with `cmux`
Track hasManuallyToggled state to prevent auto-collapse from overriding user's explicit expand actions. Once user manually toggles expansion, that preference persists even when read state changes. Generated with `cmux`
Removed hasManuallyToggled complexity. Now simply: - Mark as read → always collapses - Unmark as read → always expands - Manual toggle → persists until next read state change This is the expected behavior: marking as read should collapse regardless of previous manual expansion. Generated with `cmux`
Moved read state indicator from container border/background to header: - Removed: Green tinted background and border on container - Added: Green background (rgba(74, 222, 128, 0.15)) on header - Keeps: Gray border on container for clean, minimal look This makes read state more visually distinct from collapsed state and provides better visual separation in the hunk list. Generated with `cmux`
Changed from green to plan blue to avoid confusion with addition indicators: - Read checkmark: plan blue instead of green - Toggle read button hover: plan blue border/text - FileTree mark-as-read button: plan blue accents - Added comment: Keep header grayscale to avoid clashing with LoC Plan blue is already established in the UI for other review states, making it a natural choice for read status indication. Generated with `cmux`
- Added plan blue border to read hunks for better visual distinction - Updated collapsed text: 'Hunk marked as read. Click to expand (N lines)' - Border provides clear visual indicator even when collapsed - Text clarifies why hunk is collapsed Generated with `cmux`
Changed selected/focused hunk border from cyan blue (#007acc) to amber (#f59e0b) to prevent visual clash with plan blue read state borders. - Selected/hover: Amber border - Read state: Plan blue border - Clear visual distinction between states Generated with `cmux`
- Added --color-read and --color-read-alpha to colors.tsx - Updated all read state indicators to use centralized color - Removed mark-as-read button from FileTree (saves space) - Files with all hunks read now show blue strikethrough - Cleaner, more compact FileTree UI Generated with `cmux`
- Add --color-review-accent to colors.tsx (yellow/orange for review notes) - Add alpha variants (10%, 20%, 30%, 40%, 60%) for different use cases - Update HunkViewer to use new color variables - Remove hover border on HunkContainer to prevent clash with active border - Update DiffRenderer to use centralized color variables instead of hardcoded rgba values This ensures consistent styling across review components and makes the active hunk more visually distinct without hover interference.
When a file is selected, we only have hunk data for that file. Other files don't have their read status available, creating an ambiguous state. Now we distinguish between three states: - Known fully read: dimmed + strikethrough (blue) - Known not fully read: normal color, no strikethrough - Unknown state (no hunk data): dimmed, no strikethrough This makes it clear which files have unknown read status when filtering to a specific file.
Return null from getFileReadStatus when no hunks are loaded for a file, allowing FileTree to properly distinguish between: - Known fully read (has data, all read) - Known partially read (has data, some unread) - Unknown state (no data loaded) This fixes the dimming not working when a file is selected and other files don't have their hunks loaded.
Lower saturation from 100% to 70% for a more muted yellow/orange. This makes the active hunk border less visually jarring while still providing clear selection feedback.
- Change review accent color to use 0.75 alpha for better intensity - Remove all -alpha-10, -20, -30, etc. variants from colors.tsx - Update DiffRenderer to use inline alpha with variable reference: hsl(from var(--color-review-accent) h s l / 0.2) - Add comment documenting this as the preferred pattern to avoid cluttering color definitions with every possible alpha variant
Add second line to the review note textarea placeholder showing: 'J, K to iterate through hunks, M to toggle as read' This helps users discover navigation and marking keybinds while composing review notes.
When a user clicks on a line in a hunk to start selecting/commenting, that hunk now becomes the active one. This provides better visual feedback and ensures the active border appears on the hunk being interacted with. Added onLineClick callback that flows from SelectableDiffRenderer through HunkViewer to activate the parent hunk on line interaction.
Export evictOldestReviews from useReviewState.ts and import it in the test file instead of duplicating the implementation. Tests should verify behavior, not reimplement the logic they're testing.
When marking a hunk as read while 'Show read hunks' is off, the marked hunk gets filtered out. Now automatically moves selection to: - Next visible hunk (preferred) - Previous hunk if at end - None if this was the last hunk This prevents the selection from disappearing and allows efficient review workflow: mark as read (m) → automatically moves to next unread hunk.
Instead of checking specific conditions (!wasRead && !showReadHunks), just check if the toggled hunk will still be visible after the toggle. A hunk is visible if: showReadHunks is on OR it will be unread after toggle. This handles all cases cleanly without special-casing mark vs unmark.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
selectedHunkId is intentionally omitted from the dependency array in loadDiff effect - we only want to auto-select on initial load, not on every selection change.
Previously only hashed filePath + content, which caused collisions when multiple hunks in the same file had identical diff content (e.g., repeated code blocks). Now includes oldStart and newStart line numbers to ensure unique IDs for each hunk. This prevents marking one hunk as read from incorrectly marking all identical hunks as read.
Directories now show aggregate read status based on descendant files: - All files fully read → directory is dimmed + blue strikethrough - Any file has unknown state → directory is dimmed only - Otherwise → directory shows normal state (partially read/unread) This provides visual feedback at directory level, making it easier to track review progress through the tree structure.
Overview
Implements comprehensive read state tracking for code review hunks with visual improvements and workflow optimizations.
Read State Tracking
Visual Improvements
Color System
FileTree
Hunk Display
Workflow Improvements
Auto-navigation
Review Notes
Testing
evictOldestReviewsfrom source (no duplication in tests)Generated with
cmux