Repository navigation
v2.2.8: Bug fixes, Kanban UI improvements, and Diff Viewer performance - #67
Merged
Merged
Conversation
Updated `append_to_note` and `patch_note` functions to include the `folder` property when updating note metadata. This ensures that the folder path is preserved and correctly synced during note modification operations.
- Update KanbanColumn to use 'self-stretch' and 'h-full' to ensure consistent column heights across the board. - Adjust KanbanCard description preview to increase max height when expanded and force line clamping when collapsed. - Add CSS overrides for prose elements within cards to ensure compact vertical spacing. - Update KanbanColumn drop area to increase min-height during drag operations for better target visibility.
- Wrap major components (`HL`, `UnifiedFile`, `SplitFile`, `FileDiff`) in `React.memo` to prevent unnecessary re-renders. - Use `useMemo` for expensive calculations within `SplitFile` and `FileDiff`. - Implement `useDeferredValue` for the diff text processing in `DiffViewer` to keep the UI responsive during heavy parsing. - Refactor `KanbanBoard` zone detection to use global window coordinates instead of calculating DOM rects on every drag, improving performance and simplifying the drag interaction.
Add the changelog for version 2.2.8, documenting key fixes for file preservation and drag-and-drop state, as well as UI improvements to the Kanban board and performance optimizations for the Agent View diff viewer.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughUpdates persist note folder metadata, defer and memoize Agent diff rendering, and revise Kanban drag/drop handling with inline action targets, cancel cleanup, and size adjustments. The v2.2.8 changelog records the same changes. ChangesProduct behavior updates
Sequence Diagram(s)sequenceDiagram
participant User
participant DndContext
participant KanbanBoard
participant window
User->>DndContext: drag card
DndContext->>KanbanBoard: handleDragMove()
window->>KanbanBoard: pointermove / touchmove updates livePointer
KanbanBoard->>KanbanBoard: getZoneHit(...) and applyZoneHighlight(...)
User->>DndContext: press Escape
DndContext->>KanbanBoard: handleDragCancel()
KanbanBoard->>KanbanBoard: clear drag state and highlights
User->>DndContext: drop card
DndContext->>KanbanBoard: handleDragEnd()
KanbanBoard->>KanbanBoard: archiveCard or deleteCard
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Stylelint (17.13.0)src/app/globals.cssError: ENOENT: no such file or directory, open '/.stylelintrc.json' 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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/kanban/board.tsx (1)
141-150: 🩺 Stability & Availability | 🟠 MajorHandle touch coordinates before reading
clientX/clientY.event.activatorEventis a genericEvent; withTouchSensorthis is aTouchEvent, so the currentPointerEventcast can produceundefinedcoordinates and break archive/delete hit testing in bothhandleDragMoveandhandleDragEnd. Use a small helper that reads fromtouches[0]/changedTouches[0]for touch drags.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/kanban/board.tsx` around lines 141 - 150, `handleDragMove` (and the related `handleDragEnd` hit testing) is assuming `event.activatorEvent` is always a `PointerEvent`, which breaks touch dragging because `TouchSensor` provides a `TouchEvent`. Update the coordinate lookup in `board.tsx` to use a shared helper that reads `clientX`/`clientY` from `touches[0]` or `changedTouches[0]` for touch events and falls back to the pointer/mouse event path otherwise. Apply that helper anywhere drag coordinates are computed so archive/delete zone detection stays correct for both `handleDragMove` and `handleDragEnd`.
🧹 Nitpick comments (1)
src/components/agent/DiffFile.tsx (1)
251-303: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win
FileDiff's memo boundary is still invalidated byonToggle.
FileDiffis now memoized, butsrc/components/agent/DiffViewer.tsxstill passesonToggle={() => toggleCollapse(fileKey)}at Line 232. That fresh function prop changes on every parent render, so copy/loading/refresh updates still fan out into allFileDiffrows and blunt most of the perf win fromReact.memo.Suggested shape
// src/components/agent/DiffViewer.tsx - <FileDiff - key={fileKey} - file={file} - collapsed={collapsed.has(fileKey)} - onToggle={() => toggleCollapse(fileKey)} - mode={mode} - palette={palette} - /> + <FileDiff + key={fileKey} + file={file} + fileKey={fileKey} + collapsed={collapsed.has(fileKey)} + onToggle={toggleCollapse} + mode={mode} + palette={palette} + />// src/components/agent/DiffFile.tsx -export const FileDiff = React.memo(function FileDiff({ file, collapsed, onToggle, mode, palette }: { +export const FileDiff = React.memo(function FileDiff({ file, fileKey, collapsed, onToggle, mode, palette }: { file: File; + fileKey: string; collapsed: boolean; - onToggle: () => void; + onToggle: (key: string) => void; mode: ViewMode; palette: Palette; }) { @@ - onClick={onToggle} + onClick={() => onToggle(fileKey)}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/agent/DiffFile.tsx` around lines 251 - 303, The React.memo boundary in FileDiff is still being invalidated by a new onToggle function on every DiffViewer render. Update the DiffViewer/FileDiff wiring so FileDiff receives a stable toggle handler, ideally by memoizing the per-file toggle callback with useCallback or by passing the fileKey and handling toggle inside FileDiff, while keeping the FileDiff prop name and memoized component identity intact. Ensure the relevant symbols FileDiff and DiffViewer no longer create a fresh onToggle={() => toggleCollapse(fileKey)} for each parent render.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/agent/DiffFile.tsx`:
- Around line 193-197: The SplitFile component is reintroducing a hardcoded hex
fallback in the addBg styling, which should remain token-only to match the
themed diff color conventions. Update the addBg definition in SplitFile so it
uses only the existing CSS variable token with color-mix, consistent with delBg
and the rest of the diff UI, and remove any raw color fallback from the
expression.
---
Outside diff comments:
In `@src/components/kanban/board.tsx`:
- Around line 141-150: `handleDragMove` (and the related `handleDragEnd` hit
testing) is assuming `event.activatorEvent` is always a `PointerEvent`, which
breaks touch dragging because `TouchSensor` provides a `TouchEvent`. Update the
coordinate lookup in `board.tsx` to use a shared helper that reads
`clientX`/`clientY` from `touches[0]` or `changedTouches[0]` for touch events
and falls back to the pointer/mouse event path otherwise. Apply that helper
anywhere drag coordinates are computed so archive/delete zone detection stays
correct for both `handleDragMove` and `handleDragEnd`.
---
Nitpick comments:
In `@src/components/agent/DiffFile.tsx`:
- Around line 251-303: The React.memo boundary in FileDiff is still being
invalidated by a new onToggle function on every DiffViewer render. Update the
DiffViewer/FileDiff wiring so FileDiff receives a stable toggle handler, ideally
by memoizing the per-file toggle callback with useCallback or by passing the
fileKey and handling toggle inside FileDiff, while keeping the FileDiff prop
name and memoized component identity intact. Ensure the relevant symbols
FileDiff and DiffViewer no longer create a fresh onToggle={() =>
toggleCollapse(fileKey)} for each parent render.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 3807d29a-f21c-4ab6-8603-266d18c93d8f
📒 Files selected for processing (7)
changelogs/v2.2.8.mdelectron/mcp/tools/notes.tssrc/components/agent/DiffFile.tsxsrc/components/agent/DiffViewer.tsxsrc/components/kanban/board.tsxsrc/components/kanban/card.tsxsrc/components/kanban/column.tsx
- Refactor `FileDiff` to ingest `fileKey` and receive a callback for toggling, improving state management flexibility. - Simplify CSS color variable usage by removing redundant fallback values in `DiffFile`. - Add `pointerCoords` helper to `KanbanBoard` to correctly handle both Mouse and Touch events during drag operations. - Update `handleDragMove` and `handleDragEnd` to use the new pointer helper, preventing potential runtime errors with casting `activatorEvent`.
Refactor drag-and-drop zone detection to use DOM element bounding rectangles instead of hardcoded screen coordinates. This ensures reliable hit detection for the "archive" and "delete" zones regardless of layout changes, and implements accurate pointer tracking using a global `pointermove` listener to handle auto-scrolling during drag operations. Added associated styling for active drop zones.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/kanban/board.tsx (1)
187-206: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winKeep the delete flash renderable after dropping.
handleDragEndclearsactiveCard, but the drop-zone buttons render only whileactiveCardexists, sodeleteFlashing && "zone-active"is never visible during the 300ms delay.Proposed fix
- {activeCard && ( + {(activeCard || deleteFlashing) && ( <div className="flex items-center gap-1 ml-auto"> - <button - ref={archiveBtnRef} - type="button" - className="zone-archive flex items-center gap-1.5 px-2.5 py-1 rounded text-xs font-medium transition-colors" - > - <Archive size={11} /> - Archive - </button> + {activeCard && ( + <button + ref={archiveBtnRef} + type="button" + className="zone-archive flex items-center gap-1.5 px-2.5 py-1 rounded text-xs font-medium transition-colors" + > + <Archive size={11} /> + Archive + </button> + )} <buttonAlso applies to: 373-389
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/kanban/board.tsx` around lines 187 - 206, Keep the delete/zone flash visible after drag end by not clearing the state that gates the drop-zone rendering too early. In handleDragEnd, the immediate setActiveCard(null) prevents the delete button’s zone-active state from rendering during the 300ms timeout; preserve activeCard (or move the reset until after the delayed deleteCard flow completes) so the flash can display. Update the same drag-end/drop-zone logic in the related render path that also relies on activeCard, applyZoneHighlight, and setDeleteFlashing.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/app/globals.css`:
- Around line 557-563: The active warning/delete zone styles in globals.css use
--surface as the foreground, which is too low-contrast in the light theme.
Update the .zone-archive.zone-active and .zone-delete.zone-active rules to use
theme-aware, contrast-safe foreground tokens instead of --surface, and keep the
fix scoped to the active state styles so the Archive/Delete labels remain
readable in both themes.
In `@src/components/kanban/board.tsx`:
- Around line 35-39: The hit-test in getZoneHit is using a fixed TITLE_BAR_H
value, which can drift from the rendered title bar height under --font-scale.
Update getZoneHit and the callers around the Archive/Delete drag handling to use
the measured title-bar DOMRect Y bounds instead of the hardcoded constant, so
the zone detection matches the actual h-9 bar at any font scale.
---
Outside diff comments:
In `@src/components/kanban/board.tsx`:
- Around line 187-206: Keep the delete/zone flash visible after drag end by not
clearing the state that gates the drop-zone rendering too early. In
handleDragEnd, the immediate setActiveCard(null) prevents the delete button’s
zone-active state from rendering during the 300ms timeout; preserve activeCard
(or move the reset until after the delayed deleteCard flow completes) so the
flash can display. Update the same drag-end/drop-zone logic in the related
render path that also relies on activeCard, applyZoneHighlight, and
setDeleteFlashing.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 623fb8a6-f059-45df-a31c-b0ee2476053a
📒 Files selected for processing (2)
src/app/globals.csssrc/components/kanban/board.tsx
- Update `getZoneHit` to derive boundaries from the full `barRect` instead of a hardcoded height, increasing reliability across different viewport sizes. - Reorder state cleanup logic in `onDragEnd` to ensure `activeCard` persists during the delete action's flashing animation. - Explicitly persist zone highlight during the delete delay. - Update CSS for action zones to improve text readability on warning and danger backgrounds.
8 tasks done
14 of 17 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.
What does this PR do?
This PR includes several stability and performance improvements:
patch_noteandappend_to_notewould incorrectly move notes to the project root by failing to preserve their original folder path.useDeferredValueto prevent UI thread blocking.Type of change
Checklist
npm run type-check:allpassesnpm run lintpassesnpm testpasses (runsnpm run compilefirst soelectron/bundle-guard.test.tsactually executes —npm run test:bundleto run just that)npm run test:e2epasses (run before merging UI changes or cutting a release)var(--accent),var(--text-primary), etc.)text-[Npx]pixel font classes — rem equivalents only (text-[0.714rem],text-xs, etc.)handle()and returnIpcResult<T>schema.tselectron/db/queries.ts— single source of truth (imported by both Electron main process and MCP server); never construct aDatabaseinstance outsidedb/client.ts(Electron) ormcp-server.ts(MCP runtime)dependenciesordevDependenciesadded toROLE_MAPinscripts/generate-licenses.jsandlicenses.jsonregeneratedelectron/mcp/tools/index.tsdispatch +electron/lib/tool-schemas.tsZod schema--external:<pkg>flag in thecompilescript: updated the appropriate allowlist group (RUNTIME_PROVIDED/OPTIONAL_TRANSITIVE/SUBPROCESS_ONLY/SHIPPED_NATIVE) inelectron/bundle-guard.test.tsand (ifSHIPPED_NATIVE) shipped the package viaelectron-builder.ymlNotes for reviewer
The Diff Viewer updates use
React.memoanduseMemohooks to prevent redundant re-renders of the diff file views during polling. The Kanban changes move away from the top-banner drag drop zones to a fixed-portal bottom overlay to avoid layout shift during drag operations.Summary by CodeRabbit