feat: add plan review comment system - #1
Merged
Conversation
Implement GitHub-style code review for plan files: - Click blocks to select and add comments - Bottom bar shows all comments with line indicators - Generate consolidated review prompt - Copy to clipboard for coding agent feedback - Persistent storage in .comments.json sidecar files - Integrated clipboard support from feat/voice-to-text Backend: - Add load_comments, save_comments, hash_file Tauri commands - Add Comment and CommentsFile structs for persistence - Integrate tauri-plugin-clipboard-manager and sha2 deps Frontend: - Block selection with visual highlight - Comment CRUD operations with badges - Bottom bar UI (200px height, resizable) - Review prompt generation with unresolved filtering - Theme-aware styling for all new components UI Components: - Bottom bar with comment list and actions - Comment modal for adding feedback - Review modal with prompt preview - Line indicators (L<number>) for navigation - Badge counters on commented blocks
Event listeners were being registered before DOM elements existed, causing blank screen on app load. Moved all bottom bar and comment modal event listeners to execute after theme initialization and initial file load.
Remove devUrl from tauri.conf.json to serve static files directly from frontendDist. The app was trying to connect to http://localhost:1420 but no server was running on that port. Also add defensive checks in updateBottomBar() and selectBlock() to handle cases where DOM elements don't exist yet during initial load.
Add beforeDevCommand to start Python HTTP server on port 1420. This ensures CSS files are served with correct MIME types instead of being rejected by browser strict mode.
Replace python3 dependency with npm serve package for cross-platform compatibility. This works on any system with Node.js installed (which is already required for the project).
Tauri already serves static files automatically in dev mode. The beforeDevCommand was unnecessary complexity. Restore to original working configuration.
Tauri can serve static files directly from frontendDist without needing an external dev server. This is simpler and works correctly with proper MIME types.
- Add http-server dependency for serving static files - Configure beforeDevCommand to auto-start dev server - Remove center positioning from recording window
Window starts with visible:false and only appears when a file is opened, avoiding empty window taking up screen space.
If app starts without a file argument, show window so user can use the Open File button. Window only stays hidden if launched with a file that will trigger show() via loadFile().
The visible:false configuration was preventing window from appearing. Removed to allow normal window behavior.
- Add collapsible bottom bar for plan review comments - Support multi-selection of markdown blocks (Cmd/Ctrl+click) - Generate review prompts with quoted block content - Persist comments in .comments.json sidecar files - Add window state persistence plugin - Fix light theme icon and refresh button padding - Ignore *.comments.json files
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds an in-file markdown commenting system: frontend UI (modals, badges, bottom bar), client-side logic for comment lifecycle and selection, persistent per-markdown JSON storage, new Tauri commands (load/save comments, file hash), clipboard & window-state plugins, filesystem write capability, and small tooling/config updates. Changes
Sequence DiagramsequenceDiagram
actor User
participant Frontend as Frontend (main.js)
participant Tauri as Tauri Backend (lib.rs)
participant FS as File System
participant Clipboard as Clipboard Plugin
participant WindowState as Window-State Plugin
User->>Frontend: Open markdown file
Frontend->>Tauri: hash_file(path)
Tauri->>FS: Read file bytes
FS-->>Tauri: File bytes
Tauri-->>Frontend: SHA-256 hash
Frontend->>Tauri: load_comments(markdown_path)
Tauri->>FS: Read {markdown_path}.comments.json
alt comments file exists
FS-->>Tauri: comments JSON
Tauri-->>Frontend: CommentsFile
else missing
Tauri-->>Frontend: Default empty CommentsFile
end
Frontend->>Frontend: Render commentable blocks & badges
User->>Frontend: Add comment(s)
Frontend->>Frontend: Update comments state
User->>Frontend: Save comments
Frontend->>Tauri: save_comments(path, data)
Tauri->>FS: Write {markdown_path}.comments.json
FS-->>Tauri: Write OK
Tauri-->>Frontend: Success
User->>Frontend: Generate review prompt
Frontend->>Clipboard: copy(prompt)
Clipboard-->>Frontend: Copied
Frontend->>WindowState: persist window state (optional)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/tauri/src/main.js (1)
60-105:⚠️ Potential issue | 🟠 MajorClear stale selection when loading a new file.
selectedBlockspersists across file loads, so “Add Comment” can attach comments to unrelated blocks after switching files. Reset the selection and hide the add button when content is replaced.🧹 Suggested fix
document.getElementById("content").innerHTML = html; + + // Reset selection state for new file + selectedBlocks = []; + const addBtn = document.getElementById("bottom-bar-add-comment"); + if (addBtn) { + addBtn.style.display = "none"; + addBtn.textContent = "+ Add Comment"; + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/tauri/src/main.js` around lines 60 - 105, When loading a new file in the main file-loading flow (around where populateOutline(headings) and await loadCommentsForFile(path) are called), clear any previous selection state so comments don't attach to blocks from the previous file: reset the selectedBlocks variable (e.g., set selectedBlocks = [] or new Set()), remove any selection CSS/classes from DOM nodes (remove class like "selected" from document.querySelectorAll(".commentable-block.selected")), and hide/disable the Add Comment button (e.g., getElementById or querySelector for the add-button and set style.display = "none" or disable it) before populating the new content and loading comments; place this logic near the start of the file-load success path where headings/contents are assigned so selection is always cleared when switching files.
🧹 Nitpick comments (3)
apps/tauri/src/app.css (1)
494-498: Hardcoded color values may not adapt to theme changes.The selected block highlight uses a hardcoded
rgba(73, 163, 255, 0.15)which may not align with--linkin all themes. Consider using CSS custom properties with opacity for better theme consistency.🎨 Proposed theme-aware approach
.commentable-block.selected { - background: rgba(73, 163, 255, 0.15); + background: color-mix(in srgb, var(--link) 15%, transparent); outline: 2px solid var(--link); outline-offset: -2px; }Note:
color-mix()has broad browser support but verify it works in your WebView target. Alternatively, define a--link-bgvariable in both theme blocks.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/tauri/src/app.css` around lines 494 - 498, The selected block uses a hardcoded rgba color which can mismatch themes; update .commentable-block.selected to derive the background from theme variables instead (e.g., use color-mix() with --link or reference a new --link-bg custom property defined in both theme blocks) and keep the outline using var(--link); ensure you provide a fallback for environments without color-mix (fallback to the current rgba) and add/update the theme declarations to define --link-bg (or rely on color-mix(--link 15% on white/transparent) as appropriate) so the highlight adapts to theme changes.apps/tauri/src/index.html (1)
74-107: Consider adding accessibility attributes to interactive elements.The modal and bottom bar structure is well-organized. For improved accessibility, consider adding
aria-labelattributes to buttons that only have icons or short text, androle="dialog"witharia-modal="true"to the modal overlays.♿ Example accessibility improvements
- <div id="comment-modal" class="modal-overlay" style="display:none"> + <div id="comment-modal" class="modal-overlay" role="dialog" aria-modal="true" aria-labelledby="comment-modal-title" style="display:none"> <div class="modal-box"> - <h3>Add Comment</h3> + <h3 id="comment-modal-title">Add Comment</h3>- <button id="bottom-bar-add-comment">+ Add Comment</button> + <button id="bottom-bar-add-comment" aria-label="Add comment to selected blocks">+ Add Comment</button>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/tauri/src/index.html` around lines 74 - 107, Add ARIA attributes to the modal overlays and interactive buttons: give the modal containers (ids comment-modal and review-modal) role="dialog", aria-modal="true", aria-labelledby referencing their h3 ids (e.g., comment-modal-title, review-modal-title) and aria-describedby referencing the explanatory element ids (e.g., comment-context, review-output); add aria-hidden="true" when the modal is hidden and set it to "false" when shown. Add descriptive aria-label attributes to buttons with short text or icon-only controls (e.g., bottom-bar-add-comment, comment-cancel, comment-submit, review-close, review-copy, bottom-bar-generate) and ensure the comment-block-preview and comment-input have accessible labels (use aria-labelledby or aria-label) so screen readers can identify them.apps/tauri/src/main.js (1)
345-421: Remove debug logs from the delete flow before release.🧽 Suggested cleanup
deleteBtn.textContent = "Delete"; deleteBtn.onclick = async () => { - console.log("Delete button clicked - showing confirm"); const result = await confirm("Delete this comment?"); - console.log("Confirm result:", result); if (result) { - console.log("User clicked OK - deleting comment"); deleteComment(comment.id); - } else { - console.log("User clicked Cancel - not deleting"); } };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/tauri/src/main.js` around lines 345 - 421, In updateBottomBar's delete flow (inside the deleteBtn.onclick handler), remove the debug console.log calls so the UI doesn't emit debug output in production; specifically delete the three console.log lines ("Delete button clicked - showing confirm", "Confirm result:", result) and the subsequent "User clicked OK/Cancel" logs, leaving the existing confirm(...) await and the deleteComment(comment.id) call intact so behavior is unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/tauri/src-tauri/src/lib.rs`:
- Around line 187-199: The load_comments function currently treats any
std::fs::read_to_string error as "no comments" which hides real I/O issues;
change the match on read_to_string in load_comments to inspect the error (e.g.,
err.kind()) and only return the default CommentsFile when the error kind is
NotFound, otherwise return Err with a descriptive message including the original
error; update the error branch to propagate permission/IO errors instead of
swallowing them so callers can react to real failures.
In `@apps/tauri/src/main.js`:
- Around line 135-155: The console.warn about a file/hash mismatch in
loadCommentsForFile should be replaced with a user-visible banner or modal;
implement (or call) a helper like showStaleCommentsBanner(currentHash) from
loadCommentsForFile when commentsData.file_hash && commentsData.file_hash !==
currentHash, and ensure the banner provides a clear message and actions (e.g.,
"Reload comments" or "Ignore") and is dismissed when commentsData.file_hash is
updated or when renderCommentBadges/updateBottomBar run; add a corresponding
hideStaleCommentsBanner() and wire it to showBottomBar or to the successful
comment reload flow so the banner is removed when comments are fresh.
- Around line 221-255: The badge count is being inserted as visible text inside
blocks (renderCommentBadges -> badge.textContent), which leaks into any block
text extraction; instead set the count as a data attribute (e.g.,
badge.dataset.count = count), remove badge.textContent, mark the badge
aria-hidden (badge.setAttribute('aria-hidden','true')) and keep the badge
visually via CSS using .comment-badge::after { content: attr(data-count); } so
the DOM text nodes remain clean; additionally, ensure any code that extracts
block text (where commentsByBlock or preview code runs) strips or ignores
.comment-badge elements by removing/querying them out of a cloned node before
reading textContent.
---
Outside diff comments:
In `@apps/tauri/src/main.js`:
- Around line 60-105: When loading a new file in the main file-loading flow
(around where populateOutline(headings) and await loadCommentsForFile(path) are
called), clear any previous selection state so comments don't attach to blocks
from the previous file: reset the selectedBlocks variable (e.g., set
selectedBlocks = [] or new Set()), remove any selection CSS/classes from DOM
nodes (remove class like "selected" from
document.querySelectorAll(".commentable-block.selected")), and hide/disable the
Add Comment button (e.g., getElementById or querySelector for the add-button and
set style.display = "none" or disable it) before populating the new content and
loading comments; place this logic near the start of the file-load success path
where headings/contents are assigned so selection is always cleared when
switching files.
---
Nitpick comments:
In `@apps/tauri/src/app.css`:
- Around line 494-498: The selected block uses a hardcoded rgba color which can
mismatch themes; update .commentable-block.selected to derive the background
from theme variables instead (e.g., use color-mix() with --link or reference a
new --link-bg custom property defined in both theme blocks) and keep the outline
using var(--link); ensure you provide a fallback for environments without
color-mix (fallback to the current rgba) and add/update the theme declarations
to define --link-bg (or rely on color-mix(--link 15% on white/transparent) as
appropriate) so the highlight adapts to theme changes.
In `@apps/tauri/src/index.html`:
- Around line 74-107: Add ARIA attributes to the modal overlays and interactive
buttons: give the modal containers (ids comment-modal and review-modal)
role="dialog", aria-modal="true", aria-labelledby referencing their h3 ids
(e.g., comment-modal-title, review-modal-title) and aria-describedby referencing
the explanatory element ids (e.g., comment-context, review-output); add
aria-hidden="true" when the modal is hidden and set it to "false" when shown.
Add descriptive aria-label attributes to buttons with short text or icon-only
controls (e.g., bottom-bar-add-comment, comment-cancel, comment-submit,
review-close, review-copy, bottom-bar-generate) and ensure the
comment-block-preview and comment-input have accessible labels (use
aria-labelledby or aria-label) so screen readers can identify them.
In `@apps/tauri/src/main.js`:
- Around line 345-421: In updateBottomBar's delete flow (inside the
deleteBtn.onclick handler), remove the debug console.log calls so the UI doesn't
emit debug output in production; specifically delete the three console.log lines
("Delete button clicked - showing confirm", "Confirm result:", result) and the
subsequent "User clicked OK/Cancel" logs, leaving the existing confirm(...)
await and the deleteComment(comment.id) call intact so behavior is unchanged.
- Distinguish NotFound from other IO errors in load_comments - Replace console.warn with user-visible stale comments banner - Use data attribute + CSS ::after for badge count to prevent text leak - Clear selection state when loading a new file - Use color-mix() for theme-aware selected block highlight - Add ARIA attributes to comment and review modals - Remove debug console.log from delete flow
Contributor
Author
Code Review Feedback - All Comments AddressedAll 7 review findings from CodeRabbit have been resolved in commit 637697d. Inline Comments (3)
Outside-Diff Comment (1)
Nitpick Comments (3)
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/tauri/src-tauri/src/lib.rs`:
- Around line 211-217: The function hash_file currently reads file contents with
read_to_string which fails on non‑UTF8 files; change it to read raw bytes via
std::fs::read inside fn hash_file so you hash the raw byte slice (pass the
Vec<u8> or &[u8] to Sha256::digest) and return the hex string as before; update
the error mapping (map_err on std::fs::read) and keep the function signature and
return behavior intact.
In `@apps/tauri/src/main.js`:
- Around line 143-165: When loadCommentsForFile fails and you reset
commentsData, also clear the UI so stale entries aren't shown: inside the catch
block for loadCommentsForFile, after setting commentsData = { version: "1.0",
file_hash: "", comments: [] }, call the UI-cleanup functions (e.g.
hideStaleCommentsBanner(), hideBottomBar() or hideBottomBar equivalent,
renderCommentBadges(), updateBottomBar()) to ensure badges, banners and the
bottom bar/list reflect the cleared commentsData; reference loadCommentsForFile,
commentsData, showStaleCommentsBanner/hideStaleCommentsBanner,
renderCommentBadges, updateBottomBar, and showBottomBar to locate where to add
the calls.
Contributor
Author
Code Review Feedback - Round 2 AddressedBoth new findings from CodeRabbit (second review pass) resolved in commit 543f72f.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
apps/tauri/src/main.js (3)
172-182: Consider surfacing save errors to the user.Save failures are currently only logged to console. Users may unknowingly lose comments if saves fail silently. A brief toast or banner would improve reliability feedback.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/tauri/src/main.js` around lines 172 - 182, The catch in saveCommentsForFile currently only logs errors to console; update the catch block in saveCommentsForFile to surface failures to the user by invoking the app's UI notification method (e.g., showToast, showErrorBanner, or equivalent) with a concise message like "Failed to save comments for {currentPath}" and include error details for troubleshooting; ensure the notification is non-blocking and keep the console.error for debugging, and consider returning or propagating a failure indicator from saveCommentsForFile so callers can react if needed.
397-407: Block chip labels may be unclear to users.The
L{number}format (e.g., "L1", "L2") extracted from IDs likemkw-list-1may confuse users—does "L" mean List, Line, or Level? Consider using more descriptive prefixes (e.g., "H1" for headings, "P1" for paragraphs, "§1" generically) or showing a tooltip with the block type.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/tauri/src/main.js` around lines 397 - 407, The block chip labels use an ambiguous "L{number}" string; update the label generation in the comment.block_ids.forEach loop to derive a clearer prefix from the blockId or block element (e.g., "H" for headings, "P" for paragraphs, or "§" for generic blocks) instead of always using "L" and set chip.textContent accordingly, and add a descriptive tooltip (set chip.title) based on the resolved block type so users see the meaning on hover; locate this logic around comment.block_ids.forEach, chip.textContent, blockId.match, and chip.onclick to implement the change.
312-332: Consider using CSS variables for the padding value.The
64pxpadding is hardcoded and must stay in sync with CSS. Using a CSS variable (e.g.,var(--bottom-bar-collapsed-height)) or calculating from the element would be more maintainable.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/tauri/src/main.js` around lines 312 - 332, The showBottomBar/hideBottomBar functions use a hardcoded "64px" padding; change them to read a CSS variable from the bottom-bar element (e.g., getComputedStyle(bottomBar).getPropertyValue('--bottom-bar-collapsed-height')) and use that value for content-area.style.paddingBottom, falling back to the existing "64px" if the variable is missing or empty; ensure you trim the returned value and apply it directly (or compute px if you parse numbers) so the JS stays in sync with the bottom-bar CSS variable.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/tauri/src/main.js`:
- Around line 614-617: The truncation always appends "..." because the code
unconditionally does clone.textContent.substring(0, 100) + "..."; change the
logic in the block that handles selectedBlocks with clone/preview so you compute
the fullText = clone.textContent, then set preview.textContent to
fullText.slice(0, 100) plus "..." only when fullText.length > 100 (otherwise use
fullText as-is); update references to selectedBlocks, clone, and preview
accordingly so short content is not suffixed with ellipsis.
---
Nitpick comments:
In `@apps/tauri/src/main.js`:
- Around line 172-182: The catch in saveCommentsForFile currently only logs
errors to console; update the catch block in saveCommentsForFile to surface
failures to the user by invoking the app's UI notification method (e.g.,
showToast, showErrorBanner, or equivalent) with a concise message like "Failed
to save comments for {currentPath}" and include error details for
troubleshooting; ensure the notification is non-blocking and keep the
console.error for debugging, and consider returning or propagating a failure
indicator from saveCommentsForFile so callers can react if needed.
- Around line 397-407: The block chip labels use an ambiguous "L{number}"
string; update the label generation in the comment.block_ids.forEach loop to
derive a clearer prefix from the blockId or block element (e.g., "H" for
headings, "P" for paragraphs, or "§" for generic blocks) instead of always using
"L" and set chip.textContent accordingly, and add a descriptive tooltip (set
chip.title) based on the resolved block type so users see the meaning on hover;
locate this logic around comment.block_ids.forEach, chip.textContent,
blockId.match, and chip.onclick to implement the change.
- Around line 312-332: The showBottomBar/hideBottomBar functions use a hardcoded
"64px" padding; change them to read a CSS variable from the bottom-bar element
(e.g.,
getComputedStyle(bottomBar).getPropertyValue('--bottom-bar-collapsed-height'))
and use that value for content-area.style.paddingBottom, falling back to the
existing "64px" if the variable is missing or empty; ensure you trim the
returned value and apply it directly (or compute px if you parse numbers) so the
JS stays in sync with the bottom-bar CSS variable.
- Fix ellipsis always appended on short preview text - Surface save errors via banner instead of console only - Use descriptive block chip labels (H, P, Li, C, Q) with tooltips - Replace hardcoded padding with CSS variable
Contributor
Author
Review comments addressed in fb84418All CodeRabbit comments from the latest review round have been reviewed and resolved: Actionable fixes (implemented)
Nitpick fixes (implemented)
Already addressed in previous commits (verified)
|
There was a problem hiding this comment.
🧹 Nitpick comments (4)
apps/tauri/src/main.js (2)
434-439: Using synchronousconfirm()blocks the UI thread.The native
confirm()dialog is blocking. In a Tauri app, this works but could freeze the UI momentarily. Consider using Tauri's dialog API for consistency with the rest of the app.♻️ Consider using Tauri's confirm dialog
+const { confirm } = window.__TAURI__.dialog; + deleteBtn.onclick = async () => { - const result = await confirm("Delete this comment?"); + const result = await confirm("Delete this comment?", { + title: "Delete Comment", + kind: "warning" + }); if (result) { deleteComment(comment.id); } };Note:
confirmis already imported at line 3 as part ofopenfromdialog, so you'd need to add it to the destructuring.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/tauri/src/main.js` around lines 434 - 439, The code uses the blocking native confirm in the deleteBtn.onclick handler; update the handler to call Tauri's asynchronous confirm from the dialog API (ensure confirm is included in the destructured import alongside open) and await its promise before calling deleteComment(comment.id) so the UI is not blocked — modify the deleteBtn.onclick async handler to use the imported confirm and call deleteComment only when the awaited result is true.
179-186: Save error reuses stale-comments-banner with different semantics.The error handling correctly surfaces save failures to users (good!), but repurposing the stale-comments-banner for save errors could cause confusion if both conditions occur. Consider whether a dedicated error banner or a more generic message element would be cleaner.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/tauri/src/main.js` around lines 179 - 186, The catch block is reusing the "stale-comments-banner" for save failures which can conflict with the stale-comments state; instead create/use a dedicated banner id (e.g., "save-error-banner" or a generic "error-banner") and update the catch to select that element (banner = document.getElementById("save-error-banner")) and set its span textContent and display independently so stale-comments and save errors can show concurrently; also add the corresponding DOM element or ensure the generic error banner exists and is toggled separately from the "stale-comments-banner".apps/tauri/src/app.css (2)
296-325: Stale banner uses hardcoded colors instead of theme variables.The banner styling uses fixed colors (
#f0ad4e,#333) that won't adapt to light/dark theme changes, potentially causing accessibility or visual consistency issues in dark mode.♻️ Consider using theme-aware colors
`#stale-comments-banner` { position: fixed; top: 52px; left: 0; right: 0; display: flex; align-items: center; justify-content: center; gap: 12px; padding: 8px 16px; - background: `#f0ad4e`; - color: `#333`; + background: var(--warning-bg, `#f0ad4e`); + color: var(--warning-text, `#333`); font-size: 12px; font-weight: 500; z-index: 200; }Then define
--warning-bgand--warning-textin bothhtml.lightandhtml.darktheme blocks.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/tauri/src/app.css` around lines 296 - 325, The stale banner CSS (`#stale-comments-banner` and its button rules) uses hardcoded colors (`#f0ad4e` and `#333`); change these to theme variables (e.g., use var(--warning-bg) for background and var(--warning-text) for text and button color) and ensure you add definitions for --warning-bg and --warning-text inside your html.light and html.dark theme blocks so the banner adapts to both themes.
442-445: Hardcoded RGBA in box-shadow may not match theme.The
.highlightstate uses a hardcoded colorrgba(73, 163, 255, 0.2)which approximates--linkbut won't update if the link color changes per theme.♻️ Consider using color-mix for consistency
.bottom-bar-item.highlight { border-color: var(--link); - box-shadow: 0 0 0 2px rgba(73, 163, 255, 0.2); + box-shadow: 0 0 0 2px color-mix(in srgb, var(--link) 20%, transparent); }This mirrors the approach already used in
.commentable-block.selectedat line 530.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/tauri/src/app.css` around lines 442 - 445, The box-shadow in .bottom-bar-item.highlight uses a hardcoded rgba value; replace it with a color-mix using the --link CSS variable so the highlight tracks theme changes (follow the same pattern used in .commentable-block.selected). Locate the .bottom-bar-item.highlight rule and change the box-shadow to use color-mix(in srgb, var(--link) <percentage>, transparent) (or the exact color-mix variant used in .commentable-block.selected) so the shadow alpha is derived from --link instead of a fixed rgba.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@apps/tauri/src/app.css`:
- Around line 296-325: The stale banner CSS (`#stale-comments-banner` and its
button rules) uses hardcoded colors (`#f0ad4e` and `#333`); change these to theme
variables (e.g., use var(--warning-bg) for background and var(--warning-text)
for text and button color) and ensure you add definitions for --warning-bg and
--warning-text inside your html.light and html.dark theme blocks so the banner
adapts to both themes.
- Around line 442-445: The box-shadow in .bottom-bar-item.highlight uses a
hardcoded rgba value; replace it with a color-mix using the --link CSS variable
so the highlight tracks theme changes (follow the same pattern used in
.commentable-block.selected). Locate the .bottom-bar-item.highlight rule and
change the box-shadow to use color-mix(in srgb, var(--link) <percentage>,
transparent) (or the exact color-mix variant used in
.commentable-block.selected) so the shadow alpha is derived from --link instead
of a fixed rgba.
In `@apps/tauri/src/main.js`:
- Around line 434-439: The code uses the blocking native confirm in the
deleteBtn.onclick handler; update the handler to call Tauri's asynchronous
confirm from the dialog API (ensure confirm is included in the destructured
import alongside open) and await its promise before calling
deleteComment(comment.id) so the UI is not blocked — modify the
deleteBtn.onclick async handler to use the imported confirm and call
deleteComment only when the awaited result is true.
- Around line 179-186: The catch block is reusing the "stale-comments-banner"
for save failures which can conflict with the stale-comments state; instead
create/use a dedicated banner id (e.g., "save-error-banner" or a generic
"error-banner") and update the catch to select that element (banner =
document.getElementById("save-error-banner")) and set its span textContent and
display independently so stale-comments and save errors can show concurrently;
also add the corresponding DOM element or ensure the generic error banner exists
and is toggled separately from the "stale-comments-banner".
- Replace hardcoded banner colors with CSS variables (--warning-bg, --warning-text, --error-bg, --error-text) for light/dark themes - Create dedicated save-error-banner instead of reusing stale-comments-banner - Use Tauri dialog confirm() instead of blocking native confirm() - Replace hardcoded RGBA in .bottom-bar-item.highlight with color-mix() - Extract shared banner styles into .notification-banner base class
Contributor
Author
Addressed CodeRabbit nitpicks from latest reviewsFixed in 8221278: Review 3 nitpicks (already resolved in code before this commit):
Review 4 nitpicks (all addressed in this commit):
Review 1 nitpick (already clean):
|
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/tauri/src/app.css`:
- Around line 352-356: The CSS block for selector `#bottom-bar` has custom
properties followed immediately by a standard declaration, which Stylelint
flags; update the `#bottom-bar` rule (the block containing
--bottom-bar-collapsed-height and --bottom-bar-padding) to insert a single blank
line before the first standard declaration (position: fixed;) so there is an
empty line separating custom property declarations from regular declarations.
In `@apps/tauri/src/index.html`:
- Around line 99-109: The dynamic notification banners with ids
stale-comments-banner and save-error-banner need ARIA live regions so screen
readers announce them; update the elements (and optionally their dismiss buttons
stale-banner-dismiss and save-error-dismiss) to include an appropriate aria-live
attribute (e.g., aria-live="polite" or aria-live="assertive" depending on
importance) and ensure aria-atomic="true" so the entire message is read; also
verify that when the banners are shown/hidden the DOM text changes trigger the
live region announcement (e.g., by setting innerText or toggling display) rather
than only using CSS visibility.
In `@apps/tauri/src/main.js`:
- Around line 19-20: saveCommentsForFile currently allows concurrent disk writes
that can finish out-of-order and overwrite newer data; implement a serialization
queue for saves (e.g., a per-file promise chain or an in-memory FIFO) so each
invocation of saveCommentsForFile waits for the previous save to complete before
writing, and resolve/reject the queued promise accordingly; reference the
saveCommentsForFile function and the commentsData/selectedBlocks state and
ensure successful queued saves clear any error banner (or error flag) while
failures set it, preventing concurrent writes from clobbering newer changes.
- Queue concurrent saveCommentsForFile calls to prevent out-of-order writes from overwriting newer data - Add aria-live roles to notification banners for screen reader support - Add empty line before standard declarations in #bottom-bar CSS
- Replace parentheses with GitHub-style pill badge for comment count - Unify Generate Review button style with Add Comment button - Make review modal responsive with flexible height to prevent button clipping
- Replace height animation with transform for better performance (GPU-accelerated) - Add dedicated toggle button for bottom bar expand/collapse - Standardize all icon buttons with .icon-button class - Reduce bottom bar border to 1px and height to 46px - Normalize icon sizes to 14px across all buttons - Remove text from refresh button for consistency
- Remove readonly attribute from review prompt textarea - Change resize from none to vertical for user customization - Use code-bg background for subtle visual differentiation
This was referenced Feb 23, 2026
11 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
.comments.jsonsidecar files with file hash validationTest plan
.comments.jsonfiles are gitignored🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Chores