Add multi-select and drag functionality to sidebar #94
Conversation
- Add selection state to useUIStore with Ctrl/Cmd+click toggle and Shift+click range selection - Extract PageTreeItemContent as presentational component following dnd-kit patterns - Update SortableTree with DragOverlay for multi-select drag visualization - Add context-menu.tsx component (Radix UI based, shadcn/ui pattern) - Implement right-click context menu with batch actions for multi-select - Add batch-trash and batch-move API endpoints with permission checks - Update PageEventPayload to support batch operation events - Wire up selection interactions in PageTree and PageTreeItem
WalkthroughAdds multi-select page operations end-to-end: UI selection/store/hooks, Radix context-menu wrappers, multi-drag/render overlay in SortableTree, new BatchDeleteDialog, backend batch-trash and batch-move pageService methods and API routes, websocket batch events, and a new dependency for Radix context menus. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant UI as Page Tree UI
participant API as Batch API Route
participant Svc as Page Service
participant DB as Database
participant WS as WebSocket Broker
participant Cache as Cache/Invalidation
rect `#F0FAF0`
note over U,UI: Batch Trash (context menu)
U->>UI: Select pages + "Move to Trash"
UI->>API: POST /api/pages/batch-trash { pageIds }
API->>Svc: batchTrashPages(pageIds, userId)
Svc->>DB: authorize & fetch pages
Svc->>DB: update trashed fields (transaction)
DB-->>Svc: result { driveIds, count, aiChatPresent? }
Svc-->>API: success
API->>Cache: invalidate drive page trees
API->>WS: broadcast batch-trashed per drive
WS-->>UI: clients receive events
API-->>UI: 200 { trashedCount }
UI->>UI: clearSelection, show toast
end
rect `#F0F0FF`
note over U,UI: Batch Move (drag or menu)
U->>UI: Multi-select + drag / choose "Move"
UI->>API: POST /api/pages/batch-move { pageIds, newParentId, insertionIndex? }
API->>Svc: batchMovePages(pageIds, newParentId, userId)
Svc->>DB: authorize & fetch pages
Svc->>DB: validate moves (no cycles), compute ordering
Svc->>DB: perform moves (transaction)
DB-->>Svc: result { movedCount, targetParentId, driveIds }
Svc-->>API: success
API->>Cache: invalidate affected page trees
API->>WS: broadcast batch-moved per drive
WS-->>UI: clients receive events
API-->>UI: 200 { movedCount, targetParentId }
UI->>UI: expand target, clearSelection, show toast
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
Comment |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (7)
apps/web/src/app/api/pages/batch-move/route.ts (1)
10-14:insertionIndexis validated but unused.The schema defines
insertionIndexand it's parsed from the body, but it's never passed topageService.batchMovePages. Based on the service signature (batchMovePages(pageIds, newParentId, userId)), it doesn't accept an insertion index parameter.Either remove the field from the schema if it's not needed, or implement the insertion index functionality if this is intended for future use.
If not needed, remove from schema:
const batchMoveSchema = z.object({ pageIds: z.array(z.string()).min(1, 'At least one page ID is required'), newParentId: z.string().nullable(), - insertionIndex: z.number().int().optional(), });apps/web/src/components/layout/left-sidebar/page-tree/PageTreeItem.tsx (2)
301-316: Optimize: MultiplegetSelectedIds()calls in render.
getSelectedIds()is called multiple times in the JSX (lines 306, 314). Since this reads from the store on each call, consider storing the result in a variable at the top of the render or using auseMemo.+ const selectedIds = useMemo(() => getSelectedIds(), [getSelectedIds]); + const selectedCount = selectedIds.length; // ... ) : multiSelectActive && itemIsSelected ? ( // Multi-select context menu <> <ContextMenuItem disabled> <FolderInput className="mr-2 h-4 w-4" /> - <span>Move {getSelectedIds().length} pages...</span> + <span>Move {selectedCount} pages...</span> </ContextMenuItem> <ContextMenuSeparator /> <ContextMenuItem onSelect={handleBatchDelete} variant="destructive" > <Trash2 className="mr-2 h-4 w-4" /> - <span>Trash {getSelectedIds().length} pages</span> + <span>Trash {selectedCount} pages</span> </ContextMenuItem>
303-307: The "Move pages" menu item is disabled with no functionality.The multi-select context menu shows a disabled "Move X pages..." item. If this is a placeholder for future functionality, consider adding a TODO comment or removing it to avoid user confusion.
apps/web/src/services/api/page-service.ts (2)
638-658: Potential N+1 query pattern in authorization checks.The
Promise.allwith individualcanUserDeletePagecalls may result in N+1 queries if each call hits the database. Consider batching authorization checks if page count is expected to be large.For better performance with large selections, consider adding a batch authorization method:
// Example optimization approach const pageIdsWithAccess = await batchCheckDeletePermission(userId, pageIds); const unauthorized = pageIds.filter(id => !pageIdsWithAccess.includes(id));
776-793: Sequential updates in transaction could be slow for large batches.The transaction loop updates pages one at a time with
await. For better performance with many pages, consider using a singleinArrayupdate or batch insert pattern.- await db.transaction(async (tx) => { - for (const pageId of pageIds) { - await tx.update(pages) - .set({ - parentId: newParentId, - position: lastPosition, - updatedAt: new Date(), - }) - .where(eq(pages.id, pageId)); - lastPosition += 1; - } - }); + // Batch update with computed positions + const now = new Date(); + const updates = pageIds.map((pageId, idx) => ({ + id: pageId, + parentId: newParentId, + position: lastPosition + idx, + updatedAt: now, + })); + + await db.transaction(async (tx) => { + // Use a single bulk update if Drizzle supports it, or parallel updates + await Promise.all( + updates.map(u => + tx.update(pages) + .set({ parentId: u.parentId, position: u.position, updatedAt: u.updatedAt }) + .where(eq(pages.id, u.id)) + ) + ); + });apps/web/src/components/layout/left-sidebar/page-tree/PageTreeItemContent.tsx (2)
6-15: Remove unused imports.Several icons are imported but never used in this file:
Trash2,Pencil,Star, andUndo2. These appear to be remnants from when this component might have included inline action buttons for those operations.import { ChevronRight, Plus, MoreHorizontal, - Trash2, - Pencil, - Star, - Undo2, GripVertical, } from "lucide-react";
52-57: Consider defining a stricter type for drag handle props.The
eslint-disablecomments acknowledge the use ofanytypes here. While dnd-kit's listener and attribute types can be complex, you could improve type safety by importing or defining more specific types.Per coding guidelines, avoid
anytypes. Consider using dnd-kit's exported types:+import type { SyntheticListenerMap } from "@dnd-kit/core/dist/hooks/utilities"; +import type { DraggableAttributes } from "@dnd-kit/core"; + // Drag handle props (optional - only when draggable) dragHandleProps?: { - // eslint-disable-next-line @typescript-eslint/no-explicit-any - listeners?: Record<string, any>; - // eslint-disable-next-line @typescript-eslint/no-explicit-any - attributes?: Record<string, any>; + listeners?: SyntheticListenerMap; + attributes?: DraggableAttributes; };
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (12)
apps/web/package.json(1 hunks)apps/web/src/app/api/pages/batch-move/route.ts(1 hunks)apps/web/src/app/api/pages/batch-trash/route.ts(1 hunks)apps/web/src/components/layout/left-sidebar/page-tree/PageTree.tsx(9 hunks)apps/web/src/components/layout/left-sidebar/page-tree/PageTreeItem.tsx(7 hunks)apps/web/src/components/layout/left-sidebar/page-tree/PageTreeItemContent.tsx(1 hunks)apps/web/src/components/ui/context-menu.tsx(1 hunks)apps/web/src/components/ui/sortable-tree/SortableTree.tsx(8 hunks)apps/web/src/hooks/useUI.ts(2 hunks)apps/web/src/lib/websocket/socket-utils.ts(3 hunks)apps/web/src/services/api/page-service.ts(1 hunks)apps/web/src/stores/useUIStore.ts(5 hunks)
🧰 Additional context used
📓 Path-based instructions (8)
apps/web/src/app/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
apps/web/src/app/**/*.ts: In Next.js 15 route handlers with dynamic routes, you MUST awaitcontext.paramsbefore destructuring because params are Promise objects
Get request body usingconst body = await request.json();
Get search params usingconst { searchParams } = new URL(request.url);
Return JSON responses usingreturn Response.json(data)orreturn NextResponse.json(data)
Files:
apps/web/src/app/api/pages/batch-trash/route.tsapps/web/src/app/api/pages/batch-move/route.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Never use
anytypes in TypeScript code - always use proper TypeScript types
**/*.{ts,tsx}: Noanytypes - always use proper TypeScript types
Use kebab-case for filenames (e.g.,image-processor.ts)
Use camelCase for variables and functions
Use UPPER_SNAKE_CASE for constants
Use PascalCase for types and enums
Use centralized permission logic: importgetUserAccessLevelandcanUserEditPagefrom@pagespace/lib/permissions
Use Drizzle client from@pagespace/dbfor all database access
Always structure message content using the message parts structure:{ parts: [{ type: 'text', text: '...' }] }
Use ESM modules and TypeScript strict mode
Files:
apps/web/src/app/api/pages/batch-trash/route.tsapps/web/src/app/api/pages/batch-move/route.tsapps/web/src/components/layout/left-sidebar/page-tree/PageTreeItem.tsxapps/web/src/lib/websocket/socket-utils.tsapps/web/src/components/ui/sortable-tree/SortableTree.tsxapps/web/src/stores/useUIStore.tsapps/web/src/components/layout/left-sidebar/page-tree/PageTree.tsxapps/web/src/services/api/page-service.tsapps/web/src/hooks/useUI.tsapps/web/src/components/layout/left-sidebar/page-tree/PageTreeItemContent.tsxapps/web/src/components/ui/context-menu.tsx
apps/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
apps/**/*.{ts,tsx}: Use centralized permission functions from@pagespace/lib/permissionsfor access control, such asgetUserAccessLevel()andcanUserEditPage()
Always use Drizzle client from@pagespace/dbfor database access instead of direct database connections
Always use message parts structure withpartsarray containing objects withtypeandtextfields when constructing messages for AI
Files:
apps/web/src/app/api/pages/batch-trash/route.tsapps/web/src/app/api/pages/batch-move/route.tsapps/web/src/components/layout/left-sidebar/page-tree/PageTreeItem.tsxapps/web/src/lib/websocket/socket-utils.tsapps/web/src/components/ui/sortable-tree/SortableTree.tsxapps/web/src/stores/useUIStore.tsapps/web/src/components/layout/left-sidebar/page-tree/PageTree.tsxapps/web/src/services/api/page-service.tsapps/web/src/hooks/useUI.tsapps/web/src/components/layout/left-sidebar/page-tree/PageTreeItemContent.tsxapps/web/src/components/ui/context-menu.tsx
apps/web/src/app/**/route.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/src/app/**/route.{ts,tsx}: In Next.js 15 dynamic routes,paramsare Promise objects and must be awaited before destructuring
Get request body usingconst body = await request.json();
Return JSON responses usingResponse.json(data)orNextResponse.json(data)in route handlers
Files:
apps/web/src/app/api/pages/batch-trash/route.tsapps/web/src/app/api/pages/batch-move/route.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Format code with Prettier and lint with ESLint using the configuration at
apps/web/eslint.config.mjs
Files:
apps/web/src/app/api/pages/batch-trash/route.tsapps/web/src/app/api/pages/batch-move/route.tsapps/web/src/components/layout/left-sidebar/page-tree/PageTreeItem.tsxapps/web/src/lib/websocket/socket-utils.tsapps/web/src/components/ui/sortable-tree/SortableTree.tsxapps/web/src/stores/useUIStore.tsapps/web/src/components/layout/left-sidebar/page-tree/PageTree.tsxapps/web/src/services/api/page-service.tsapps/web/src/hooks/useUI.tsapps/web/src/components/layout/left-sidebar/page-tree/PageTreeItemContent.tsxapps/web/src/components/ui/context-menu.tsx
apps/web/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/src/**/*.{ts,tsx}: Use Zustand for client-side state management
Use SWR for server state and caching
Files:
apps/web/src/app/api/pages/batch-trash/route.tsapps/web/src/app/api/pages/batch-move/route.tsapps/web/src/components/layout/left-sidebar/page-tree/PageTreeItem.tsxapps/web/src/lib/websocket/socket-utils.tsapps/web/src/components/ui/sortable-tree/SortableTree.tsxapps/web/src/stores/useUIStore.tsapps/web/src/components/layout/left-sidebar/page-tree/PageTree.tsxapps/web/src/services/api/page-service.tsapps/web/src/hooks/useUI.tsapps/web/src/components/layout/left-sidebar/page-tree/PageTreeItemContent.tsxapps/web/src/components/ui/context-menu.tsx
apps/web/src/**/*.tsx
📄 CodeRabbit inference engine (CLAUDE.md)
apps/web/src/**/*.tsx: For document editing, register editing state usinguseEditingStore.getState().startEditing()andendEditing()to prevent unwanted UI refreshes
For AI streaming operations, register streaming state usinguseEditingStore.getState().startStreaming()andendStreaming()to prevent unwanted UI refreshes
When using SWR, checkuseEditingStorestate withisAnyActive()and setisPausedto prevent data refreshes during editing or streaming
Files:
apps/web/src/components/layout/left-sidebar/page-tree/PageTreeItem.tsxapps/web/src/components/ui/sortable-tree/SortableTree.tsxapps/web/src/components/layout/left-sidebar/page-tree/PageTree.tsxapps/web/src/components/layout/left-sidebar/page-tree/PageTreeItemContent.tsxapps/web/src/components/ui/context-menu.tsx
**/{components,src/**/components}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use PascalCase for React component names and filenames
Files:
apps/web/src/components/layout/left-sidebar/page-tree/PageTreeItem.tsxapps/web/src/components/ui/sortable-tree/SortableTree.tsxapps/web/src/components/layout/left-sidebar/page-tree/PageTree.tsxapps/web/src/components/layout/left-sidebar/page-tree/PageTreeItemContent.tsxapps/web/src/components/ui/context-menu.tsx
🧠 Learnings (9)
📚 Learning: 2025-12-14T14:54:47.103Z
Learnt from: CR
Repo: 2witstudios/PageSpace PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-14T14:54:47.103Z
Learning: Applies to apps/web/src/app/**/route.{ts,tsx} : Get request body using `const body = await request.json();`
Applied to files:
apps/web/src/app/api/pages/batch-trash/route.tsapps/web/src/app/api/pages/batch-move/route.ts
📚 Learning: 2025-12-14T14:54:45.713Z
Learnt from: CR
Repo: 2witstudios/PageSpace PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-14T14:54:45.713Z
Learning: Applies to app/api/**/*.{ts,tsx} : Use `const body = await request.json();` to extract request bodies, `const { searchParams } = new URL(request.url);` for query parameters, and `return Response.json(data)` or `return NextResponse.json(data)` for responses
Applied to files:
apps/web/src/app/api/pages/batch-trash/route.ts
📚 Learning: 2025-12-14T14:54:47.103Z
Learnt from: CR
Repo: 2witstudios/PageSpace PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-14T14:54:47.103Z
Learning: Tech stack: Next.js 15 App Router + TypeScript + Tailwind + shadcn/ui
Applied to files:
apps/web/src/components/layout/left-sidebar/page-tree/PageTreeItem.tsxapps/web/src/components/layout/left-sidebar/page-tree/PageTree.tsxapps/web/src/components/layout/left-sidebar/page-tree/PageTreeItemContent.tsx
📚 Learning: 2025-12-14T14:54:47.103Z
Learnt from: CR
Repo: 2witstudios/PageSpace PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-14T14:54:47.103Z
Learning: Applies to apps/web/src/**/*.{ts,tsx} : Use Zustand for client-side state management
Applied to files:
apps/web/src/components/layout/left-sidebar/page-tree/PageTreeItem.tsxapps/web/src/stores/useUIStore.tsapps/web/src/components/layout/left-sidebar/page-tree/PageTree.tsx
📚 Learning: 2025-12-14T14:54:15.308Z
Learnt from: CR
Repo: 2witstudios/PageSpace PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T14:54:15.308Z
Learning: Applies to apps/web/src/**/*.tsx : For document editing, register editing state using `useEditingStore.getState().startEditing()` and `endEditing()` to prevent unwanted UI refreshes
Applied to files:
apps/web/src/stores/useUIStore.tsapps/web/src/hooks/useUI.ts
📚 Learning: 2025-12-14T14:54:47.103Z
Learnt from: CR
Repo: 2witstudios/PageSpace PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-14T14:54:47.103Z
Learning: Applies to apps/web/src/**/*.{ts,tsx} : Use SWR for server state and caching
Applied to files:
apps/web/src/components/layout/left-sidebar/page-tree/PageTree.tsx
📚 Learning: 2025-12-14T14:54:45.713Z
Learnt from: CR
Repo: 2witstudios/PageSpace PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-14T14:54:45.713Z
Learning: Applies to **/*.{ts,tsx} : Always use the Drizzle client and database exports from `pagespace/db` (e.g., `import { db, pages } from 'pagespace/db'`) for all database access
Applied to files:
apps/web/src/components/layout/left-sidebar/page-tree/PageTree.tsx
📚 Learning: 2025-12-14T14:54:45.713Z
Learnt from: CR
Repo: 2witstudios/PageSpace PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-14T14:54:45.713Z
Learning: Applies to app/**/*.{ts,tsx} : Import and use `getUserAccessLevel()` and `canUserEditPage()` from `pagespace/lib/permissions` for centralized permission logic
Applied to files:
apps/web/src/hooks/useUI.ts
📚 Learning: 2025-12-14T14:54:15.308Z
Learnt from: CR
Repo: 2witstudios/PageSpace PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T14:54:15.308Z
Learning: Applies to apps/**/*.{ts,tsx} : Use centralized permission functions from `pagespace/lib/permissions` for access control, such as `getUserAccessLevel()` and `canUserEditPage()`
Applied to files:
apps/web/src/hooks/useUI.ts
🧬 Code graph analysis (3)
apps/web/src/app/api/pages/batch-move/route.ts (2)
apps/web/src/lib/auth/index.ts (2)
authenticateRequestWithOptions(216-271)isAuthError(204-206)apps/web/src/lib/websocket/socket-utils.ts (2)
broadcastPageEvent(91-125)createPageEventPayload(217-237)
apps/web/src/components/layout/left-sidebar/page-tree/PageTreeItem.tsx (4)
apps/web/src/hooks/useUI.ts (1)
usePageSelection(86-137)apps/web/src/components/layout/left-sidebar/page-tree/PageTreeItemContent.tsx (2)
DropPosition(20-20)PageTreeItemContent(60-265)apps/web/src/stores/useUIStore.ts (1)
SelectionMode(4-4)apps/web/src/components/ui/context-menu.tsx (5)
ContextMenu(240-240)ContextMenuTrigger(241-241)ContextMenuContent(242-242)ContextMenuItem(243-243)ContextMenuSeparator(247-247)
apps/web/src/hooks/useUI.ts (1)
apps/web/src/stores/useUIStore.ts (2)
useUIStore(43-179)SelectionMode(4-4)
🔇 Additional comments (36)
apps/web/package.json (1)
33-33: LGTM!Standard addition of the Radix UI context menu package, following the existing pattern of other Radix UI dependencies in the project.
apps/web/src/hooks/useUI.ts (2)
1-2: LGTM!Import correctly added for the new
SelectionModetype.
85-137: LGTM!The
usePageSelectionhook follows the established patterns in this file and provides a clean API for multi-select functionality. The use of individual Zustand selectors ensures components only re-render when their specific slice of state changes.apps/web/src/app/api/pages/batch-trash/route.ts (3)
1-12: LGTM!Auth configuration with CSRF protection and Zod schema validation follow established patterns. The schema correctly enforces at least one page ID.
30-47: LGTM!The broadcast and cache invalidation logic correctly handles all affected drives. Using
pageIds[0]as the primarypageIdwhile including the full list inpageIdsis a sensible approach given thePageEventPayloadinterface requirements.
53-65: LGTM!Error handling properly distinguishes validation errors (400) from server errors (500) and includes appropriate logging.
apps/web/src/app/api/pages/batch-move/route.ts (2)
16-51: LGTM!Handler implementation correctly follows the established patterns: authentication with CSRF, service delegation, broadcasting to affected drives, and cache invalidation.
52-64: LGTM!Error handling is consistent with the batch-trash route and properly categorizes validation vs server errors.
apps/web/src/lib/websocket/socket-utils.ts (3)
14-14: LGTM!Clean extension of
PageOperationtype to support batch operations while maintaining backward compatibility with existing operations.
28-30: LGTM!Optional batch fields are well-documented and maintain backward compatibility with existing event payloads.
217-236: LGTM!The
createPageEventPayloadhelper cleanly extends to support batch operation fields via the options parameter, maintaining consistency with the interface definition.apps/web/src/components/layout/left-sidebar/page-tree/PageTree.tsx (4)
186-211: Good implementation of batch move with toast feedback.The
handleMultiMovecorrectly auto-expands the target parent, shows a loading toast, and handles errors. One consideration: if any individual page move fails server-side, the current implementation treats it as a total failure. Ensure the API returns partial success information if needed for better UX.
336-342: LGTM: Drag overlay render callback is clean.The
renderDragOverlaycallback correctly delegates toDragOverlayContentwith items and count. The empty dependency array is appropriate since it only uses the passed props.
363-391: Verify new props are correctly typed in SortableTree.The new props
selectedIds,onMultiMove,onSelectionChange,renderDragOverlay, andflattenedIdsare passed toSortableTreeandPageTreeItem. Ensure the prop types match the expected interfaces.
73-88: Event listener may not correctly intercept context menu interactions due to selector mismatch and dependency instability.The selector
[data-slot="context-menu"]targets the Root element, but the actual portaled context menu content renders withdata-slot="context-menu-content"inside ContextMenuPrimitive.Portal. Additionally, sinceusePageSelection()returns a new object on each render without memoization, theclearSelectiondependency causes unnecessary effect re-runs. Consider either matching the selector to the actual rendered portal content ([data-slot="context-menu-content"]) or memoizing the hook's return value to stabilize the dependency array.apps/web/src/components/layout/left-sidebar/page-tree/PageTreeItem.tsx (2)
194-211: Selection click handler logic is correct.The
handleSelectionClickcorrectly determines selection mode based on modifier keys and passesflattenedIdsfor range selection. ThestopPropagationandpreventDefaultprevent unintended navigation.
234-245: Good UX: Auto-select on context menu if item not selected.When right-clicking an unselected item, clearing selection and selecting that single item provides expected behavior matching file explorer patterns.
apps/web/src/components/ui/sortable-tree/SortableTree.tsx (4)
114-127: Multi-drag children removal logic is correct.When multiple items are selected and being dragged, the code correctly removes children of all selected items from the sortable list to prevent invalid drop targets. The fallback to single-item removal when only one item is involved maintains backward compatibility.
157-170: Selection sync on drag start improves UX.When dragging an unselected item, the selection is updated to only that item via
onSelectionChange. This matches expected behavior in file managers and prevents confusion about what's being dragged.
196-209: Multi-drag end logic correctly routes to appropriate handler.The
handleDragEndproperly distinguishes between single and multi-drag scenarios, callingonMultiMovewith all selected IDs for multi-drag oronMovefor single drag.
265-273: DragOverlay renders correctly for multi-select.The
DragOverlaywithdropAnimation={null}prevents jarring animations during drag operations. The conditional rendering ensures the overlay only appears when actively dragging and a render function is provided.apps/web/src/services/api/page-service.ts (2)
668-670: Strict count validation may reject valid partial operations.If some pages were already deleted between the authorization check and the fetch, the
pagesInfo.length !== pageIds.lengthcheck returns a 404. Consider whether partial success is acceptable for batch operations.
809-843: Type definitions are well-structured.The
BatchTrashResultandBatchMoveResultdiscriminated unions follow the existing pattern in the file for success/error result types. Good consistency.apps/web/src/stores/useUIStore.ts (4)
4-4: SelectionMode type is well-defined.The
SelectionModetype with 'single', 'toggle', and 'range' options covers standard multi-select interaction patterns.
95-136: Selection logic correctly implements all three modes.The
selectPagefunction properly handles:
- single: Clears and selects only the clicked item
- toggle: Ctrl/Cmd+click behavior to add/remove from selection
- range: Shift+click behavior with fallback when no previous selection exists
The range selection correctly handles bidirectional ranges (startIdx > endIdx case).
118-135: Range selection preserves existing selection, which may not match expected behavior.In many file managers, Shift+click replaces the selection with the new range rather than adding to it. The current implementation adds the range to existing selection (
newSelectedstarts as a copy ofselectedPageIds). Verify this is the intended behavior.
165-175: Good decision: Selection state not persisted.Resetting selection on rehydration is the correct choice. Persisting selection across sessions could lead to stale references to deleted pages.
apps/web/src/components/layout/left-sidebar/page-tree/PageTreeItemContent.tsx (2)
273-291: LGTM!The
DragOverlayContentcomponent handles single-item drag preview cleanly with appropriate null check for empty arrays.
294-317: Multi-item drag overlay provides good visual feedback.The stacked card effect with count badge effectively communicates multi-select drag operations. The visual hierarchy is clear.
apps/web/src/components/ui/context-menu.tsx (7)
1-7: LGTM!Imports follow the standard shadcn/ui pattern with Radix primitives, lucide icons, and the
cnutility.
9-57: LGTM!These thin wrapper components correctly forward props to their Radix counterparts while adding consistent
data-slotattributes for styling hooks and testing.
59-81: LGTM!
ContextMenuSubTriggercorrectly implements the inset pattern and appends the chevron icon for submenu indication. The styling handles focus, open state, and inset variants appropriately.
99-115: LGTM!
ContextMenuContentproperly wraps content in a Portal and applies comprehensive animation/transition classes for smooth open/close states.
117-138: LGTM!
ContextMenuItemcorrectly implements thevariantprop for destructive styling and handles disabled states appropriately via data attributes.
140-188: LGTM!Checkbox and radio items follow the Radix pattern correctly with proper indicator positioning and icon rendering.
239-255: LGTM!All components are properly exported, providing a complete context menu API surface.
apps/web/src/components/layout/left-sidebar/page-tree/PageTreeItem.tsx
Outdated
Show resolved
Hide resolved
apps/web/src/components/layout/left-sidebar/page-tree/PageTreeItemContent.tsx
Outdated
Show resolved
Hide resolved
apps/web/src/components/layout/left-sidebar/page-tree/PageTreeItemContent.tsx
Outdated
Show resolved
Hide resolved
| } else { | ||
| // Moving to root - use the first page's drive | ||
| targetDriveId = pagesInfo[0]?.drive?.id || ''; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, find the page-service.ts file and examine the relevant code section
fd "page-service.ts" --type fRepository: 2witstudios/PageSpace
Length of output: 106
🏁 Script executed:
# Find and examine the page-service.ts file
fd "page-service.ts" --type f -x cat -n {}Repository: 2witstudios/PageSpace
Length of output: 29361
🏁 Script executed:
# Find the page-service.ts file
fd "page-service.ts" --type fRepository: 2witstudios/PageSpace
Length of output: 106
🏁 Script executed:
# Examine the batchMovePages function and related code
rg -n "batchMovePages" --type=ts -A 20Repository: 2witstudios/PageSpace
Length of output: 3367
🏁 Script executed:
# Search for cross-drive or same-drive validation logic
rg -n "drive" apps/web/src/services/api/page-service.ts -B 5 -A 5 | head -100Repository: 2witstudios/PageSpace
Length of output: 2775
🏁 Script executed:
# Check the batch-move route endpoint for validation
cat apps/web/src/app/api/pages/batch-move/route.tsRepository: 2witstudios/PageSpace
Length of output: 2288
🏁 Script executed:
# Search for any cross-drive validation or restrictions
rg -n "driveId" apps/web/src/app/api/pages/batch-move/ -B 3 -A 3Repository: 2witstudios/PageSpace
Length of output: 1429
🏁 Script executed:
# Check if there's validation elsewhere preventing cross-drive moves
rg -n "same.*drive|cross.*drive|multiple.*drive" --type=ts apps/web/src/Repository: 2witstudios/PageSpace
Length of output: 5469
🏁 Script executed:
# Check the page-write-tools for same-drive validation context
sed -n '670,690p' apps/web/src/lib/ai/tools/page-write-tools.tsRepository: 2witstudios/PageSpace
Length of output: 770
🏁 Script executed:
# Search for any enforced cross-drive checks in the codebase
rg -n "same drive|same-drive" --type=ts -B 2 -A 2Repository: 2witstudios/PageSpace
Length of output: 473
🏁 Script executed:
# Get more context around the same-drive validation in page-write-tools
sed -n '670,700p' apps/web/src/lib/ai/tools/page-write-tools.tsRepository: 2witstudios/PageSpace
Length of output: 1189
Add validation to prevent cross-drive batch moves or fix position calculation
When newParentId is null, the code uses the first page's drive to calculate positions for all moved pages. This assumes all pages are from the same drive, but there's no enforcement preventing cross-drive batch moves. If pages from multiple drives are moved to root, the position calculation only accounts for siblings in the first page's drive, producing incorrect positions for pages from other drives.
Note: The AI page-write tool enforces same-drive moves explicitly with eq(pages.driveId, page.driveId), but the batch API lacks this validation. Either add a check preventing cross-drive moves at the API level (consistent with the AI tool), or update the position calculation to group pages by drive.
- Remove unused useParams import and params variable from PageTreeItem - Remove unused event parameter from handleContextMenu callback - Remove unused lucide-react imports (Trash2, Pencil, Star, Undo2) from PageTreeItemContent - Add void statements for interface props that are passed but handled by context menu
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
apps/web/src/components/layout/left-sidebar/page-tree/PageTreeItem.tsx (1)
142-155: Batch delete needs confirmation and children handling.This was previously flagged: batch delete immediately trashes multiple pages without confirmation, unlike single-item delete which opens
DeletePageDialog. Additionally, the batch operation doesn't handle the case where selected pages might have children, which could lead to unexpected data loss.Consider:
- Adding a confirmation dialog (as previously suggested)
- Checking if any selected pages have children and prompting accordingly
🧹 Nitpick comments (2)
apps/web/src/components/layout/left-sidebar/page-tree/PageTreeItem.tsx (2)
101-101: Consider simplifying the hasChildren check.The condition
item.children && item.children.length > 0can be simplified toitem.children?.length > 0using optional chaining, which is more concise and equally safe.Apply this diff:
- const hasChildren = item.children && item.children.length > 0; + const hasChildren = (item.children?.length ?? 0) > 0;
302-305: Remove or implement the disabled Move option.Showing a disabled "Move X pages..." option in the context menu may confuse users—they'll wonder why it's visible but unusable. Either implement the batch-move functionality or remove this menu item until it's ready.
To remove the disabled item until implementation:
- <ContextMenuItem disabled> - <FolderInput className="mr-2 h-4 w-4" /> - <span>Move {getSelectedIds().length} pages...</span> - </ContextMenuItem> - <ContextMenuSeparator /> <ContextMenuItem onSelect={handleBatchDelete} variant="destructive"
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/web/src/components/layout/left-sidebar/page-tree/PageTreeItem.tsx(7 hunks)apps/web/src/components/layout/left-sidebar/page-tree/PageTreeItemContent.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web/src/components/layout/left-sidebar/page-tree/PageTreeItemContent.tsx
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Never use
anytypes in TypeScript code - always use proper TypeScript types
**/*.{ts,tsx}: Noanytypes - always use proper TypeScript types
Use kebab-case for filenames (e.g.,image-processor.ts)
Use camelCase for variables and functions
Use UPPER_SNAKE_CASE for constants
Use PascalCase for types and enums
Use centralized permission logic: importgetUserAccessLevelandcanUserEditPagefrom@pagespace/lib/permissions
Use Drizzle client from@pagespace/dbfor all database access
Always structure message content using the message parts structure:{ parts: [{ type: 'text', text: '...' }] }
Use ESM modules and TypeScript strict mode
Files:
apps/web/src/components/layout/left-sidebar/page-tree/PageTreeItem.tsx
apps/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
apps/**/*.{ts,tsx}: Use centralized permission functions from@pagespace/lib/permissionsfor access control, such asgetUserAccessLevel()andcanUserEditPage()
Always use Drizzle client from@pagespace/dbfor database access instead of direct database connections
Always use message parts structure withpartsarray containing objects withtypeandtextfields when constructing messages for AI
Files:
apps/web/src/components/layout/left-sidebar/page-tree/PageTreeItem.tsx
apps/web/src/**/*.tsx
📄 CodeRabbit inference engine (CLAUDE.md)
apps/web/src/**/*.tsx: For document editing, register editing state usinguseEditingStore.getState().startEditing()andendEditing()to prevent unwanted UI refreshes
For AI streaming operations, register streaming state usinguseEditingStore.getState().startStreaming()andendStreaming()to prevent unwanted UI refreshes
When using SWR, checkuseEditingStorestate withisAnyActive()and setisPausedto prevent data refreshes during editing or streaming
Files:
apps/web/src/components/layout/left-sidebar/page-tree/PageTreeItem.tsx
**/{components,src/**/components}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use PascalCase for React component names and filenames
Files:
apps/web/src/components/layout/left-sidebar/page-tree/PageTreeItem.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Format code with Prettier and lint with ESLint using the configuration at
apps/web/eslint.config.mjs
Files:
apps/web/src/components/layout/left-sidebar/page-tree/PageTreeItem.tsx
apps/web/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/src/**/*.{ts,tsx}: Use Zustand for client-side state management
Use SWR for server state and caching
Files:
apps/web/src/components/layout/left-sidebar/page-tree/PageTreeItem.tsx
🧠 Learnings (3)
📚 Learning: 2025-12-14T14:54:47.103Z
Learnt from: CR
Repo: 2witstudios/PageSpace PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-14T14:54:47.103Z
Learning: Tech stack: Next.js 15 App Router + TypeScript + Tailwind + shadcn/ui
Applied to files:
apps/web/src/components/layout/left-sidebar/page-tree/PageTreeItem.tsx
📚 Learning: 2025-12-14T14:54:47.103Z
Learnt from: CR
Repo: 2witstudios/PageSpace PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-14T14:54:47.103Z
Learning: Applies to apps/web/src/**/*.{ts,tsx} : Use Zustand for client-side state management
Applied to files:
apps/web/src/components/layout/left-sidebar/page-tree/PageTreeItem.tsx
📚 Learning: 2025-12-14T14:54:15.308Z
Learnt from: CR
Repo: 2witstudios/PageSpace PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T14:54:15.308Z
Learning: Applies to apps/web/src/**/*.tsx : For document editing, register editing state using `useEditingStore.getState().startEditing()` and `endEditing()` to prevent unwanted UI refreshes
Applied to files:
apps/web/src/components/layout/left-sidebar/page-tree/PageTreeItem.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Unit Tests
- Create BatchDeleteDialog component with page count display - Show confirmation before batch trashing multiple selected pages - Dialog displays count and optional list of page names (up to 5) - User can confirm or cancel the batch delete action
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
apps/web/src/components/layout/left-sidebar/page-tree/PageTreeItem.tsx (1)
230-232: Ensuredata-selection-checkboxattribute is added to PageTreeItemContent.The click handler checks for a parent with
[data-selection-checkbox](line 230), but per previous review comments, the selection indicator div inPageTreeItemContent.tsxis missing this attribute. This prevents proper checkbox click detection.Verify that the selection indicator div in
PageTreeItemContent.tsxincludes thedata-selection-checkboxattribute as previously requested.
🧹 Nitpick comments (2)
apps/web/src/components/dialogs/BatchDeleteDialog.tsx (1)
42-44: Consider using a more stable key for list items.Using array indices as keys is acceptable for this read-only display list, but using the name itself or a combination would follow React best practices more closely.
Apply this diff:
- {pageNames.map((name, index) => ( - <li key={index} className="truncate">{name}</li> + {pageNames.map((name) => ( + <li key={name} className="truncate">{name}</li> ))}Note: This assumes page names are unique within the selection. If duplicates are possible, use
key={${name}-${index}}instead.apps/web/src/components/layout/left-sidebar/page-tree/PageTreeItem.tsx (1)
314-329: Optional: Cache selected count for render efficiency.Lines 319 and 327 both call
getSelectedIds().lengthduring render. While not a critical issue, consider computing this once for minor efficiency gains.+ const selectedCount = multiSelectActive ? getSelectedIds().length : 0; + // Multi-select context menu <> <ContextMenuItem disabled> <FolderInput className="mr-2 h-4 w-4" /> - <span>Move {getSelectedIds().length} pages...</span> + <span>Move {selectedCount} pages...</span> </ContextMenuItem> <ContextMenuSeparator /> <ContextMenuItem onSelect={handleBatchDelete} variant="destructive" > <Trash2 className="mr-2 h-4 w-4" /> - <span>Trash {getSelectedIds().length} pages</span> + <span>Trash {selectedCount} pages</span> </ContextMenuItem> </>
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/web/src/components/dialogs/BatchDeleteDialog.tsx(1 hunks)apps/web/src/components/layout/left-sidebar/page-tree/PageTreeItem.tsx(8 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Never use
anytypes in TypeScript code - always use proper TypeScript types
**/*.{ts,tsx}: Noanytypes - always use proper TypeScript types
Use kebab-case for filenames (e.g.,image-processor.ts)
Use camelCase for variables and functions
Use UPPER_SNAKE_CASE for constants
Use PascalCase for types and enums
Use centralized permission logic: importgetUserAccessLevelandcanUserEditPagefrom@pagespace/lib/permissions
Use Drizzle client from@pagespace/dbfor all database access
Always structure message content using the message parts structure:{ parts: [{ type: 'text', text: '...' }] }
Use ESM modules and TypeScript strict mode
Files:
apps/web/src/components/layout/left-sidebar/page-tree/PageTreeItem.tsxapps/web/src/components/dialogs/BatchDeleteDialog.tsx
apps/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
apps/**/*.{ts,tsx}: Use centralized permission functions from@pagespace/lib/permissionsfor access control, such asgetUserAccessLevel()andcanUserEditPage()
Always use Drizzle client from@pagespace/dbfor database access instead of direct database connections
Always use message parts structure withpartsarray containing objects withtypeandtextfields when constructing messages for AI
Files:
apps/web/src/components/layout/left-sidebar/page-tree/PageTreeItem.tsxapps/web/src/components/dialogs/BatchDeleteDialog.tsx
apps/web/src/**/*.tsx
📄 CodeRabbit inference engine (CLAUDE.md)
apps/web/src/**/*.tsx: For document editing, register editing state usinguseEditingStore.getState().startEditing()andendEditing()to prevent unwanted UI refreshes
For AI streaming operations, register streaming state usinguseEditingStore.getState().startStreaming()andendStreaming()to prevent unwanted UI refreshes
When using SWR, checkuseEditingStorestate withisAnyActive()and setisPausedto prevent data refreshes during editing or streaming
Files:
apps/web/src/components/layout/left-sidebar/page-tree/PageTreeItem.tsxapps/web/src/components/dialogs/BatchDeleteDialog.tsx
**/{components,src/**/components}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use PascalCase for React component names and filenames
Files:
apps/web/src/components/layout/left-sidebar/page-tree/PageTreeItem.tsxapps/web/src/components/dialogs/BatchDeleteDialog.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Format code with Prettier and lint with ESLint using the configuration at
apps/web/eslint.config.mjs
Files:
apps/web/src/components/layout/left-sidebar/page-tree/PageTreeItem.tsxapps/web/src/components/dialogs/BatchDeleteDialog.tsx
apps/web/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/src/**/*.{ts,tsx}: Use Zustand for client-side state management
Use SWR for server state and caching
Files:
apps/web/src/components/layout/left-sidebar/page-tree/PageTreeItem.tsxapps/web/src/components/dialogs/BatchDeleteDialog.tsx
🧠 Learnings (3)
📚 Learning: 2025-12-14T14:54:47.103Z
Learnt from: CR
Repo: 2witstudios/PageSpace PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-14T14:54:47.103Z
Learning: Tech stack: Next.js 15 App Router + TypeScript + Tailwind + shadcn/ui
Applied to files:
apps/web/src/components/layout/left-sidebar/page-tree/PageTreeItem.tsx
📚 Learning: 2025-12-14T14:54:47.103Z
Learnt from: CR
Repo: 2witstudios/PageSpace PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-14T14:54:47.103Z
Learning: Applies to apps/web/src/**/*.{ts,tsx} : Use Zustand for client-side state management
Applied to files:
apps/web/src/components/layout/left-sidebar/page-tree/PageTreeItem.tsx
📚 Learning: 2025-12-14T14:54:15.308Z
Learnt from: CR
Repo: 2witstudios/PageSpace PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T14:54:15.308Z
Learning: Applies to apps/web/src/**/*.tsx : For document editing, register editing state using `useEditingStore.getState().startEditing()` and `endEditing()` to prevent unwanted UI refreshes
Applied to files:
apps/web/src/components/layout/left-sidebar/page-tree/PageTreeItem.tsx
🧬 Code graph analysis (1)
apps/web/src/components/layout/left-sidebar/page-tree/PageTreeItem.tsx (5)
apps/web/src/hooks/useUI.ts (1)
usePageSelection(86-137)apps/web/src/components/layout/left-sidebar/page-tree/PageTreeItemContent.tsx (2)
DropPosition(16-16)PageTreeItemContent(56-270)apps/web/src/stores/useUIStore.ts (1)
SelectionMode(4-4)apps/web/src/components/ui/context-menu.tsx (5)
ContextMenu(240-240)ContextMenuTrigger(241-241)ContextMenuContent(242-242)ContextMenuItem(243-243)ContextMenuSeparator(247-247)apps/web/src/components/dialogs/BatchDeleteDialog.tsx (1)
BatchDeleteDialog(20-64)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Unit Tests
🔇 Additional comments (3)
apps/web/src/components/layout/left-sidebar/page-tree/PageTreeItem.tsx (3)
1-64: LGTM: Clean imports and interface additions.The new imports for batch operations, context menu components, and selection hooks are well-organized. The
flattenedIdsoptional parameter properly supports range selection functionality.
360-387: LGTM: Dialogs properly integrated.All dialog components are correctly wired with appropriate state management. The addition of
BatchDeleteDialogaddresses the previous concern about confirmation for batch operations. TheDropPositionre-export maintains backwards compatibility.
82-107: Parent component correctly providesflattenedIdsfor range selection.The
PageTreecomponent passesflattenedIdsfrom theSortableTreerender callback toPageTreeItem(line 381 in PageTree.tsx). This enables range selection to function properly when users hold Shift and click. The empty array default is appropriate for toggle-only mode, and the parent ensures it's populated when the tree renders.
| pageCount, | ||
| pageNames, | ||
| }: BatchDeleteDialogProps) { | ||
| const showNames = pageNames && pageNames.length > 0 && pageNames.length <= 5; |
There was a problem hiding this comment.
Fix the confusing "and N more..." message when no names are displayed.
When there are more than 5 pages, showNames is false (line 27), so no page names are rendered in lines 40-46. However, lines 47-51 then display "and {pageNames.length - 5} more..." which implies that some names were already shown. This creates a confusing user experience.
Solution 1 (recommended): Display the first 5 names plus "and N more..." when there are more than 5 pages:
- const showNames = pageNames && pageNames.length > 0 && pageNames.length <= 5;
+ const showNames = pageNames && pageNames.length > 0;
+ const displayNames = showNames ? pageNames.slice(0, 5) : []; {showNames && (
<ul className="mt-2 list-disc list-inside text-sm text-muted-foreground">
- {pageNames.map((name, index) => (
+ {displayNames.map((name, index) => (
<li key={index} className="truncate">{name}</li>
))}
</ul>Solution 2: Change the message to not imply previous names were shown:
{pageNames && pageNames.length > 5 && (
<p className="text-sm text-muted-foreground">
- and {pageNames.length - 5} more...
+ {pageNames.length} pages selected
</p>
)}Also applies to: 40-51
🤖 Prompt for AI Agents
In apps/web/src/components/dialogs/BatchDeleteDialog.tsx around line 27 (and
adjust rendering at lines 40-51): the current boolean showNames is false when
pageNames.length > 5 which prevents rendering any names while still showing "and
N more...", causing confusion; change the logic to render the first five names
when there are more than five (i.e., compute a displayedNames =
pageNames.slice(0, 5) and set showNames to displayedNames.length > 0), update
the JSX at lines 40-46 to map over displayedNames to show those names, and keep
the "and {pageNames.length - displayedNames.length} more..." text only when
pageNames.length > displayedNames.length; alternatively, if you prefer Solution
2, change the trailing message to "and {pageNames.length} more..." and only show
it when no names are displayed—implement Solution 1 by default.
| const showNames = pageNames && pageNames.length > 0 && pageNames.length <= 5; | ||
|
|
||
| return ( | ||
| <AlertDialog open={isOpen} onOpenChange={onClose}> |
There was a problem hiding this comment.
Wrap the onClose callback to match onOpenChange signature.
The onOpenChange prop expects (open: boolean) => void, but onClose is typed as () => void. While this may work at runtime, it's not type-safe and could cause issues with strict TypeScript configurations.
Apply this diff:
- <AlertDialog open={isOpen} onOpenChange={onClose}>
+ <AlertDialog open={isOpen} onOpenChange={(open) => !open && onClose()}>📝 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.
| <AlertDialog open={isOpen} onOpenChange={onClose}> | |
| <AlertDialog open={isOpen} onOpenChange={(open) => !open && onClose()}> |
🤖 Prompt for AI Agents
In apps/web/src/components/dialogs/BatchDeleteDialog.tsx around line 30, the
AlertDialog is passing onClose (type () => void) directly to onOpenChange which
expects (open: boolean) => void; wrap onClose in a handler that matches the
expected signature, e.g. create a const handleOpenChange = (open: boolean) => {
if (!open) onClose?.(); } and pass handleOpenChange to onOpenChange so
TypeScript types align and closing only triggers when open becomes false.
| const handleBatchDelete = () => { | ||
| const selectedIds = getSelectedIds(); | ||
| if (selectedIds.length === 0) return; | ||
|
|
||
| // Open confirmation dialog with selected page info | ||
| setBatchDeleteInfo({ ids: selectedIds, count: selectedIds.length }); | ||
| setIsBatchDeleteOpen(true); | ||
| }; | ||
|
|
||
| const confirmBatchDelete = async () => { | ||
| const { ids, count } = batchDeleteInfo; | ||
| if (ids.length === 0) return; | ||
|
|
||
| setIsBatchDeleteOpen(false); | ||
| const toastId = toast.loading(`Moving ${count} pages to trash...`); | ||
| try { | ||
| await post("/api/pages/batch-trash", { pageIds: ids }); | ||
| clearSelection(); | ||
| await mutate(); | ||
| toast.success(`${count} pages moved to trash.`, { id: toastId }); | ||
| } catch { | ||
| toast.error("Error moving pages to trash.", { id: toastId }); | ||
| } finally { | ||
| setBatchDeleteInfo({ ids: [], count: 0 }); | ||
| } | ||
| }; |
There was a problem hiding this comment.
Consider permission checks and children handling for batch operations.
Two concerns with the batch delete implementation:
-
Missing permission checks: No frontend validation before batch operations. Per coding guidelines, use centralized permission functions from
@pagespace/lib/permissions(e.g.,canUserEditPage()) to pre-validate that the user can delete all selected pages. This prevents partial failures and provides better UX. -
Children handling: Single-page delete (via
DeletePageDialogat line 364) asks whether to trash children. Batch delete doesn't handle this case. If any selected page has children, the behavior is unclear.
To add permission checks, import and use the centralized functions:
import { canUserEditPage } from "@pagespace/lib/permissions";
const handleBatchDelete = () => {
const selectedIds = getSelectedIds();
if (selectedIds.length === 0) return;
// Validate permissions for all selected pages
// (assuming access to page data and user context)
setBatchDeleteInfo({ ids: selectedIds, count: selectedIds.length });
setIsBatchDeleteOpen(true);
};For children handling, consider enhancing BatchDeleteDialog to accept a hasChildrenPages prop and display the trash-children option similar to DeletePageDialog.
🤖 Prompt for AI Agents
In apps/web/src/components/layout/left-sidebar/page-tree/PageTreeItem.tsx around
lines 145-170, the batch delete flow lacks permission checks and handling for
pages that have children; update handleBatchDelete to import and use the
centralized permission helpers (e.g., canUserEditPage) to validate that the
current user can delete every selected page and abort with a user-facing message
if any selection is not permitted, and detect whether any selected page has
children (via the existing page data/map or a lightweight API call) and pass
that state into the batch delete dialog (or add a hasChildrenPages prop) so the
dialog can present the "trash children" option; finally, ensure
confirmBatchDelete includes the chosen "trash children" option in the POST
payload to /api/pages/batch-trash and maintain the existing success/error/toast
and cleanup behavior.
- Convert selection indicator from div to semantic button element - Add aria-pressed to indicate selection state - Add aria-label describing the action (Select/Deselect page title) - Add onKeyDown handler for Enter/Space key support - Add focus-visible ring styles for keyboard navigation - Reset native button styles to preserve visual appearance - Update type signatures to accept MouseEvent | KeyboardEvent
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
apps/web/src/components/layout/left-sidebar/page-tree/PageTreeItem.tsx (1)
145-170: Permission checks and children handling for batch operations.This is a known concern from a previous review. The batch delete operation doesn't validate permissions client-side before attempting the operation, and doesn't handle pages with children (unlike single-page delete which prompts for this via
DeletePageDialog).Per coding guidelines, consider using centralized permission functions from
@pagespace/lib/permissionsto pre-validate operations. The backend should enforce permissions regardless, but client-side validation improves UX by catching issues early.apps/web/src/components/layout/left-sidebar/page-tree/PageTreeItemContent.tsx (1)
249-258: "More options" button has an empty click handler.The onClick handler only calls
e.stopPropagation()and does nothing else. This was flagged in a previous review. The context menu now handles actions, so consider either:
- Removing this button entirely since right-click provides the same functionality
- Wiring it to programmatically open the context menu
🧹 Nitpick comments (2)
apps/web/src/components/layout/left-sidebar/page-tree/PageTreeItem.tsx (1)
314-329: Avoid callinggetSelectedIds()multiple times during render.
getSelectedIds()is called twice within the JSX (lines 319 and 327). Since this function likely iterates over the selection state, consider calling it once at the beginning of the render function and reusing the result.+ const selectedIds = getSelectedIds(); + const selectedCount = selectedIds.length; + const multiSelectActive = selectedCount > 1; // In the JSX: ) : multiSelectActive && itemIsSelected ? ( // Multi-select context menu <> <ContextMenuItem disabled> <FolderInput className="mr-2 h-4 w-4" /> - <span>Move {getSelectedIds().length} pages...</span> + <span>Move {selectedCount} pages...</span> </ContextMenuItem> <ContextMenuSeparator /> <ContextMenuItem onSelect={handleBatchDelete} variant="destructive" > <Trash2 className="mr-2 h-4 w-4" /> - <span>Trash {getSelectedIds().length} pages</span> + <span>Trash {selectedCount} pages</span> </ContextMenuItem> </>apps/web/src/components/layout/left-sidebar/page-tree/PageTreeItemContent.tsx (1)
93-100: Consider removing unused prop destructuring.Six props are destructured but immediately silenced with
void. While the comment explains this is for "interface completeness," it adds noise. Consider either:
- Not destructuring these props (use
_propNamein parameter destructuring pattern)- Using a single rest spread if truly unused:
...rest- isFavorite: _isFavorite = false, + isFavorite: _isFavorite, isHovered = false, isTrashView = false, style, onToggleExpand, onOpenCreateDialog, - onRename: _onRename, - onTrash: _onTrash, - onRestore: _onRestore, - onPermanentDelete: _onPermanentDelete, - onFavoriteToggle: _onFavoriteToggle, + onRename: _onRename, + onTrash: _onTrash, + onRestore: _onRestore, + onPermanentDelete: _onPermanentDelete, + onFavoriteToggle: _onFavoriteToggle, onMouseEnter, onMouseLeave, onClick, onContextMenu, onSelectionClick, dragHandleProps, }, ref ) { const params = useParams(); const linkHref = `/dashboard/${params.driveId}/${item.id}`; const effectiveIndicatorDepth = indicatorDepth ?? depth; - - // Silence unused vars - these callbacks are passed for interface completeness - // but actions are handled via context menu in PageTreeItem - void _isFavorite; - void _onRename; - void _onTrash; - void _onRestore; - void _onPermanentDelete; - void _onFavoriteToggle;The underscore prefix convention already tells TypeScript/ESLint these are intentionally unused.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/web/src/components/layout/left-sidebar/page-tree/PageTreeItem.tsx(8 hunks)apps/web/src/components/layout/left-sidebar/page-tree/PageTreeItemContent.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Never use
anytypes in TypeScript code - always use proper TypeScript types
**/*.{ts,tsx}: Noanytypes - always use proper TypeScript types
Use kebab-case for filenames (e.g.,image-processor.ts)
Use camelCase for variables and functions
Use UPPER_SNAKE_CASE for constants
Use PascalCase for types and enums
Use centralized permission logic: importgetUserAccessLevelandcanUserEditPagefrom@pagespace/lib/permissions
Use Drizzle client from@pagespace/dbfor all database access
Always structure message content using the message parts structure:{ parts: [{ type: 'text', text: '...' }] }
Use ESM modules and TypeScript strict mode
Files:
apps/web/src/components/layout/left-sidebar/page-tree/PageTreeItem.tsxapps/web/src/components/layout/left-sidebar/page-tree/PageTreeItemContent.tsx
apps/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
apps/**/*.{ts,tsx}: Use centralized permission functions from@pagespace/lib/permissionsfor access control, such asgetUserAccessLevel()andcanUserEditPage()
Always use Drizzle client from@pagespace/dbfor database access instead of direct database connections
Always use message parts structure withpartsarray containing objects withtypeandtextfields when constructing messages for AI
Files:
apps/web/src/components/layout/left-sidebar/page-tree/PageTreeItem.tsxapps/web/src/components/layout/left-sidebar/page-tree/PageTreeItemContent.tsx
apps/web/src/**/*.tsx
📄 CodeRabbit inference engine (CLAUDE.md)
apps/web/src/**/*.tsx: For document editing, register editing state usinguseEditingStore.getState().startEditing()andendEditing()to prevent unwanted UI refreshes
For AI streaming operations, register streaming state usinguseEditingStore.getState().startStreaming()andendStreaming()to prevent unwanted UI refreshes
When using SWR, checkuseEditingStorestate withisAnyActive()and setisPausedto prevent data refreshes during editing or streaming
Files:
apps/web/src/components/layout/left-sidebar/page-tree/PageTreeItem.tsxapps/web/src/components/layout/left-sidebar/page-tree/PageTreeItemContent.tsx
**/{components,src/**/components}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use PascalCase for React component names and filenames
Files:
apps/web/src/components/layout/left-sidebar/page-tree/PageTreeItem.tsxapps/web/src/components/layout/left-sidebar/page-tree/PageTreeItemContent.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Format code with Prettier and lint with ESLint using the configuration at
apps/web/eslint.config.mjs
Files:
apps/web/src/components/layout/left-sidebar/page-tree/PageTreeItem.tsxapps/web/src/components/layout/left-sidebar/page-tree/PageTreeItemContent.tsx
apps/web/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/src/**/*.{ts,tsx}: Use Zustand for client-side state management
Use SWR for server state and caching
Files:
apps/web/src/components/layout/left-sidebar/page-tree/PageTreeItem.tsxapps/web/src/components/layout/left-sidebar/page-tree/PageTreeItemContent.tsx
🧠 Learnings (7)
📚 Learning: 2025-12-14T14:54:15.308Z
Learnt from: CR
Repo: 2witstudios/PageSpace PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T14:54:15.308Z
Learning: Applies to apps/**/*.{ts,tsx} : Use centralized permission functions from `pagespace/lib/permissions` for access control, such as `getUserAccessLevel()` and `canUserEditPage()`
Applied to files:
apps/web/src/components/layout/left-sidebar/page-tree/PageTreeItem.tsx
📚 Learning: 2025-12-14T14:54:47.103Z
Learnt from: CR
Repo: 2witstudios/PageSpace PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-14T14:54:47.103Z
Learning: Applies to **/*.{ts,tsx} : Use centralized permission logic: import `getUserAccessLevel` and `canUserEditPage` from `pagespace/lib/permissions`
Applied to files:
apps/web/src/components/layout/left-sidebar/page-tree/PageTreeItem.tsx
📚 Learning: 2025-12-14T14:54:38.009Z
Learnt from: CR
Repo: 2witstudios/PageSpace PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-14T14:54:38.009Z
Learning: Applies to apps/**/*.ts : Always use centralized permission functions from `pagespace/lib/permissions` for access control logic (e.g., `getUserAccessLevel`, `canUserEditPage`)
Applied to files:
apps/web/src/components/layout/left-sidebar/page-tree/PageTreeItem.tsx
📚 Learning: 2025-12-14T14:54:45.713Z
Learnt from: CR
Repo: 2witstudios/PageSpace PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-14T14:54:45.713Z
Learning: Applies to app/**/*.{ts,tsx} : Import and use `getUserAccessLevel()` and `canUserEditPage()` from `pagespace/lib/permissions` for centralized permission logic
Applied to files:
apps/web/src/components/layout/left-sidebar/page-tree/PageTreeItem.tsx
📚 Learning: 2025-12-14T14:54:47.103Z
Learnt from: CR
Repo: 2witstudios/PageSpace PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-14T14:54:47.103Z
Learning: Tech stack: Next.js 15 App Router + TypeScript + Tailwind + shadcn/ui
Applied to files:
apps/web/src/components/layout/left-sidebar/page-tree/PageTreeItem.tsxapps/web/src/components/layout/left-sidebar/page-tree/PageTreeItemContent.tsx
📚 Learning: 2025-12-14T14:54:47.103Z
Learnt from: CR
Repo: 2witstudios/PageSpace PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-14T14:54:47.103Z
Learning: Applies to apps/web/src/**/*.{ts,tsx} : Use Zustand for client-side state management
Applied to files:
apps/web/src/components/layout/left-sidebar/page-tree/PageTreeItem.tsx
📚 Learning: 2025-12-14T14:54:15.308Z
Learnt from: CR
Repo: 2witstudios/PageSpace PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T14:54:15.308Z
Learning: Applies to apps/web/src/**/*.tsx : For document editing, register editing state using `useEditingStore.getState().startEditing()` and `endEditing()` to prevent unwanted UI refreshes
Applied to files:
apps/web/src/components/layout/left-sidebar/page-tree/PageTreeItem.tsx
🧬 Code graph analysis (2)
apps/web/src/components/layout/left-sidebar/page-tree/PageTreeItem.tsx (4)
apps/web/src/components/layout/left-sidebar/page-tree/PageTreeItemContent.tsx (2)
DropPosition(16-16)PageTreeItemContent(56-282)apps/web/src/stores/useUIStore.ts (1)
SelectionMode(4-4)apps/web/src/components/ui/context-menu.tsx (5)
ContextMenu(240-240)ContextMenuTrigger(241-241)ContextMenuContent(242-242)ContextMenuItem(243-243)ContextMenuSeparator(247-247)apps/web/src/components/dialogs/BatchDeleteDialog.tsx (1)
BatchDeleteDialog(20-64)
apps/web/src/components/layout/left-sidebar/page-tree/PageTreeItemContent.tsx (2)
apps/web/src/hooks/usePageTree.ts (1)
TreePage(35-40)apps/web/src/components/common/PageTypeIcon.tsx (1)
PageTypeIcon(34-45)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Unit Tests
🔇 Additional comments (6)
apps/web/src/components/layout/left-sidebar/page-tree/PageTreeItem.tsx (2)
317-320: The "Move pages" menu item is disabled with no implementation.The move functionality is shown as disabled. Consider either hiding it until implemented or adding a TODO comment to track this work.
Is there an open issue tracking the implementation of batch move functionality? If not, consider creating one to ensure this feature gets completed.
260-295: Good extraction of PageTreeItemContent with proper prop forwarding.The component correctly forwards the ref, separates presentation from behavior, and properly passes all necessary handlers. The ContextMenu wrapping pattern is clean and follows Radix UI conventions.
apps/web/src/components/layout/left-sidebar/page-tree/PageTreeItemContent.tsx (4)
144-177: Good keyboard accessibility implementation for selection checkbox.The selection indicator correctly uses a
<button>element with:
onKeyDownhandler for Enter/Space keysaria-pressedstate- Descriptive
aria-labelwith item titleThis addresses the accessibility concern from the previous review.
290-335: Well-designed DragOverlayContent component.The component cleanly handles both single-item and multi-item drag scenarios with:
- Appropriate visual feedback (stacked cards effect for multi-select)
- Badge showing total count
- Reuse of
PageTypeIconfor consistencyMinor note: The early return on line 292 handles the edge case of an empty items array gracefully.
220-227: Link navigation correctly stops event propagation.Good pattern - the
onClickwithe.stopPropagation()prevents the row's click handler from interfering with normal link navigation while still allowing modifier+click selection behavior to work at the row level.
48-53: Replaceanytypes with proper dnd-kit type definitions.Per coding guidelines, avoid
anytypes. Use specific types from @dnd-kit instead:import type { UseSortableReturnValue } from '@dnd-kit/sortable'; import type { DraggableAttributes } from '@dnd-kit/core'; dragHandleProps?: Pick<UseSortableReturnValue, 'listeners' | 'attributes'>;Or, if you prefer individual types:
import type { DraggableAttributes } from '@dnd-kit/core'; dragHandleProps?: { listeners?: Record<string, Function>; attributes?: DraggableAttributes; };
- Cache getSelectedIds() call to avoid multiple iterations during render - Use cached selectedCount instead of calling getSelectedIds().length - Remove unused isMultiSelect from hook destructuring - Remove empty "More options" button (context menu serves same purpose) - Remove unused MoreHorizontal import
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
apps/web/src/components/layout/left-sidebar/page-tree/PageTreeItem.tsx (1)
146-170: Add permission validation and children handling for batch operations.Two issues remain from previous review feedback:
Missing permission checks: No validation that the user can edit all selected pages before attempting batch delete. Per coding guidelines and learnings, you must use centralized permission functions from
@pagespace/lib/permissions.No children handling: Unlike single-page delete (line 364) which has a
hasChildrenprop for the dialog, batch delete doesn't detect or handle pages with children. Users expect the same "trash children" option as single delete.Import and use the centralized permission helper:
+import { canUserEditPage } from "@pagespace/lib/permissions"; + // ...inside PageTreeItem function... const handleBatchDelete = async () => { if (selectedCount === 0) return; + + // TODO: Add permission validation for all selected pages + // Need access to user context and page data to call canUserEditPage() + // for each selected page and show error if any are not editable + + // TODO: Detect if any selected pages have children + // Pass hasChildrenPages flag to BatchDeleteDialog + // Update BatchDeleteDialog to show trash-children option like DeletePageDialog setBatchDeleteInfo({ ids: selectedIds, count: selectedCount }); setIsBatchDeleteOpen(true); };Then update
confirmBatchDeleteto include the trash-children choice in the API payload when applicable.As per coding guidelines, always use centralized permission logic from
@pagespace/lib/permissions.
🧹 Nitpick comments (1)
apps/web/src/components/layout/left-sidebar/page-tree/PageTreeItemContent.tsx (1)
93-98: Remove unused props or document why they're in the interface.These props are deliberately silenced with
voidstatements and never used in the component. If they're "passed for interface completeness" but not needed for rendering, consider either:
- Removing them from the interface entirely
- Making them truly optional and omitting the void statements
- Adding clearer documentation about why they exist
The current pattern adds cognitive load without benefit. Since PageTreeItem handles these actions through context menu callbacks, these props aren't needed in PageTreeItemContent:
export interface PageTreeItemContentProps { // ... - onRename?: () => void; - onTrash?: () => void; - onRestore?: () => void; - onPermanentDelete?: () => void; - onFavoriteToggle?: () => void; // ... }Then remove lines 93-98 entirely.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/web/src/components/layout/left-sidebar/page-tree/PageTreeItem.tsx(8 hunks)apps/web/src/components/layout/left-sidebar/page-tree/PageTreeItemContent.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Never use
anytypes in TypeScript code - always use proper TypeScript types
**/*.{ts,tsx}: Noanytypes - always use proper TypeScript types
Use kebab-case for filenames (e.g.,image-processor.ts)
Use camelCase for variables and functions
Use UPPER_SNAKE_CASE for constants
Use PascalCase for types and enums
Use centralized permission logic: importgetUserAccessLevelandcanUserEditPagefrom@pagespace/lib/permissions
Use Drizzle client from@pagespace/dbfor all database access
Always structure message content using the message parts structure:{ parts: [{ type: 'text', text: '...' }] }
Use ESM modules and TypeScript strict mode
Files:
apps/web/src/components/layout/left-sidebar/page-tree/PageTreeItemContent.tsxapps/web/src/components/layout/left-sidebar/page-tree/PageTreeItem.tsx
apps/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
apps/**/*.{ts,tsx}: Use centralized permission functions from@pagespace/lib/permissionsfor access control, such asgetUserAccessLevel()andcanUserEditPage()
Always use Drizzle client from@pagespace/dbfor database access instead of direct database connections
Always use message parts structure withpartsarray containing objects withtypeandtextfields when constructing messages for AI
Files:
apps/web/src/components/layout/left-sidebar/page-tree/PageTreeItemContent.tsxapps/web/src/components/layout/left-sidebar/page-tree/PageTreeItem.tsx
apps/web/src/**/*.tsx
📄 CodeRabbit inference engine (CLAUDE.md)
apps/web/src/**/*.tsx: For document editing, register editing state usinguseEditingStore.getState().startEditing()andendEditing()to prevent unwanted UI refreshes
For AI streaming operations, register streaming state usinguseEditingStore.getState().startStreaming()andendStreaming()to prevent unwanted UI refreshes
When using SWR, checkuseEditingStorestate withisAnyActive()and setisPausedto prevent data refreshes during editing or streaming
Files:
apps/web/src/components/layout/left-sidebar/page-tree/PageTreeItemContent.tsxapps/web/src/components/layout/left-sidebar/page-tree/PageTreeItem.tsx
**/{components,src/**/components}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use PascalCase for React component names and filenames
Files:
apps/web/src/components/layout/left-sidebar/page-tree/PageTreeItemContent.tsxapps/web/src/components/layout/left-sidebar/page-tree/PageTreeItem.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Format code with Prettier and lint with ESLint using the configuration at
apps/web/eslint.config.mjs
Files:
apps/web/src/components/layout/left-sidebar/page-tree/PageTreeItemContent.tsxapps/web/src/components/layout/left-sidebar/page-tree/PageTreeItem.tsx
apps/web/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/src/**/*.{ts,tsx}: Use Zustand for client-side state management
Use SWR for server state and caching
Files:
apps/web/src/components/layout/left-sidebar/page-tree/PageTreeItemContent.tsxapps/web/src/components/layout/left-sidebar/page-tree/PageTreeItem.tsx
🧠 Learnings (7)
📚 Learning: 2025-12-14T14:54:47.103Z
Learnt from: CR
Repo: 2witstudios/PageSpace PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-14T14:54:47.103Z
Learning: Tech stack: Next.js 15 App Router + TypeScript + Tailwind + shadcn/ui
Applied to files:
apps/web/src/components/layout/left-sidebar/page-tree/PageTreeItemContent.tsxapps/web/src/components/layout/left-sidebar/page-tree/PageTreeItem.tsx
📚 Learning: 2025-12-14T14:54:15.308Z
Learnt from: CR
Repo: 2witstudios/PageSpace PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T14:54:15.308Z
Learning: Applies to apps/**/*.{ts,tsx} : Use centralized permission functions from `pagespace/lib/permissions` for access control, such as `getUserAccessLevel()` and `canUserEditPage()`
Applied to files:
apps/web/src/components/layout/left-sidebar/page-tree/PageTreeItem.tsx
📚 Learning: 2025-12-14T14:54:47.103Z
Learnt from: CR
Repo: 2witstudios/PageSpace PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-14T14:54:47.103Z
Learning: Applies to **/*.{ts,tsx} : Use centralized permission logic: import `getUserAccessLevel` and `canUserEditPage` from `pagespace/lib/permissions`
Applied to files:
apps/web/src/components/layout/left-sidebar/page-tree/PageTreeItem.tsx
📚 Learning: 2025-12-14T14:54:38.009Z
Learnt from: CR
Repo: 2witstudios/PageSpace PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-14T14:54:38.009Z
Learning: Applies to apps/**/*.ts : Always use centralized permission functions from `pagespace/lib/permissions` for access control logic (e.g., `getUserAccessLevel`, `canUserEditPage`)
Applied to files:
apps/web/src/components/layout/left-sidebar/page-tree/PageTreeItem.tsx
📚 Learning: 2025-12-14T14:54:45.713Z
Learnt from: CR
Repo: 2witstudios/PageSpace PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-14T14:54:45.713Z
Learning: Applies to app/**/*.{ts,tsx} : Import and use `getUserAccessLevel()` and `canUserEditPage()` from `pagespace/lib/permissions` for centralized permission logic
Applied to files:
apps/web/src/components/layout/left-sidebar/page-tree/PageTreeItem.tsx
📚 Learning: 2025-12-14T14:54:47.103Z
Learnt from: CR
Repo: 2witstudios/PageSpace PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-14T14:54:47.103Z
Learning: Applies to apps/web/src/**/*.{ts,tsx} : Use Zustand for client-side state management
Applied to files:
apps/web/src/components/layout/left-sidebar/page-tree/PageTreeItem.tsx
📚 Learning: 2025-12-14T14:54:15.308Z
Learnt from: CR
Repo: 2witstudios/PageSpace PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T14:54:15.308Z
Learning: Applies to apps/web/src/**/*.tsx : For document editing, register editing state using `useEditingStore.getState().startEditing()` and `endEditing()` to prevent unwanted UI refreshes
Applied to files:
apps/web/src/components/layout/left-sidebar/page-tree/PageTreeItem.tsx
🧬 Code graph analysis (2)
apps/web/src/components/layout/left-sidebar/page-tree/PageTreeItemContent.tsx (3)
apps/web/src/components/layout/left-sidebar/page-tree/PageTreeItem.tsx (1)
DropPosition(387-387)apps/web/src/hooks/usePageTree.ts (1)
TreePage(35-40)apps/web/src/components/common/PageTypeIcon.tsx (1)
PageTypeIcon(34-45)
apps/web/src/components/layout/left-sidebar/page-tree/PageTreeItem.tsx (5)
apps/web/src/hooks/useUI.ts (1)
usePageSelection(86-137)apps/web/src/components/layout/left-sidebar/page-tree/PageTreeItemContent.tsx (2)
DropPosition(15-15)PageTreeItemContent(55-269)apps/web/src/stores/useUIStore.ts (1)
SelectionMode(4-4)apps/web/src/components/ui/context-menu.tsx (5)
ContextMenu(240-240)ContextMenuTrigger(241-241)ContextMenuContent(242-242)ContextMenuItem(243-243)ContextMenuSeparator(247-247)apps/web/src/components/dialogs/BatchDeleteDialog.tsx (1)
BatchDeleteDialog(20-64)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Unit Tests
🔇 Additional comments (6)
apps/web/src/components/layout/left-sidebar/page-tree/PageTreeItemContent.tsx (2)
142-175: Well-implemented accessible selection control!The selection checkbox now uses a semantic
<button>element with proper keyboard support (Enter/Space keys), ARIA attributes (aria-pressed,aria-label), and the requireddata-selection-checkboxattribute. This fully addresses the accessibility concerns from previous reviews.
277-322: Clean implementation of drag overlay visuals.The
DragOverlayContentcomponent properly handles both single-item and multi-item drag scenarios with appropriate visual feedback. The stacked card effect for multiple items and the count badge provide clear UX.apps/web/src/components/layout/left-sidebar/page-tree/PageTreeItem.tsx (4)
208-224: Selection click handler properly implements multi-select patterns.The handler correctly maps modifier keys to selection modes (Shift for range, Ctrl/Cmd for toggle) and uses
useCallbackwith appropriate dependencies for performance.
227-245: Row click logic correctly delegates to selection and navigation.The handler properly distinguishes between selection checkbox clicks (via
data-selection-checkbox), modifier-key selection, and normal navigation. The early return pattern prevents conflicts.
248-258: Context menu handler enables intuitive multi-select actions.The logic correctly ensures that right-clicking an unselected item selects it first (single mode), while preserving existing multi-selection when right-clicking an already-selected item. This provides the expected UX for batch operations.
299-357: Context menu provides appropriate actions for each scenario.The conditional menu structure properly handles trash view, multi-select, and single-item contexts. The disabled "Move" option for multi-select is clearly a placeholder for future functionality.
apps/web/src/components/layout/left-sidebar/page-tree/PageTreeItemContent.tsx
Show resolved
Hide resolved
- Import DraggableAttributes and DraggableSyntheticListeners from @dnd-kit/core - Replace Record<string, any> with proper typed interfaces - Remove eslint-disable comments for @typescript-eslint/no-explicit-any
Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.