[web] Fix recents tracking and mobile dropdown#378
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
📝 WalkthroughWalkthroughRoute-driven side effects were removed from the dashboard Page component and consolidated into CenterPanel. CenterPanel now derives activeDriveId, syncs activePageId to the store, posts page-view events and clears hasChanges. RecentsDropdown now fetches recents via SWR and the Recents API types page.type as PageType. Changes
Sequence Diagram(s)sequenceDiagram
participant Router as Router
participant Center as CenterPanel
participant Store as Global Store / PageTree
participant API as /api/pages/:id/view
participant Update as updateNode (usePageTree)
Router->>Center: route change (driveId, pageId)
Center->>Store: set activeDriveId(activeDriveId)
Center->>Store: set activePageId(activePageId)
Center->>Update: clear hasChanges for pageId (via updateNodeRef)
Center->>API: POST /api/pages/{activePageId}/view
API-->>Center: 200 OK
Center->>Update: confirm clear hasChanges (on success)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@apps/web/src/components/shared/RecentsDropdown.tsx`:
- Around line 80-87: The component RecentsDropdown currently treats any
fetch/auth error the same as an empty recents list; update the render branch
that checks error || !data?.recents || data.recents.length === 0 to separate the
error case: if error is truthy, render a distinct error UI (e.g., "Failed to
load recent pages" with optional retry control) and only fall back to "No recent
pages" when data.recents exists but is empty; locate the conditional using
isLoading, error, and data?.recents in RecentsDropdown to implement this change.
- Around line 96-103: The RecentPage.type is currently typed as string but
should use the PageType enum; update the RecentPage definition in
apps/web/src/app/api/user/recents/route.ts to declare type: PageType (and import
PageType there), then in components like RecentsDropdown.tsx remove the
unnecessary "as PageType" assertion (e.g., where PageTypeIcon is used) and run
TS checks to update any other usages of RecentPage to the new PageType-typed
field.
🧹 Nitpick comments (1)
apps/web/src/components/shared/RecentsDropdown.tsx (1)
52-59: Add SWR editing-protection pause to prevent revalidation interruptions during editing.The fetch has a 5-minute refresh interval that will continue during user editing. Add
isPausedwith editing state protection, consistent with patterns used elsewhere in the codebase. The coding guidelines specify: useisPaused: () => hasLoadedRef.current && isEditingActive()to allow the initial fetch, then pause revalidation after load.Pattern example from InboxCenterList.tsx
const hasLoadedRef = useRef(false); const { data, isLoading, error } = useSWR(/* ... */, { // ... other options isPaused: () => hasLoadedRef.current && isEditingActive(), onSuccess: () => { hasLoadedRef.current = true; }, });
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/src/app/api/user/recents/route.ts (1)
23-26:⚠️ Potential issue | 🟡 MinorGuard against invalid
limitvalues producingNaN.
parseIntcan returnNaN, which will propagate throughMath.min/Math.maxand can break the DB query. Use a safe fallback when parsing fails.Suggested fix
- const limit = Math.min(Math.max(parseInt(limitParam || '8', 10), 1), 50); + const parsedLimit = Number.parseInt(limitParam ?? '8', 10); + const safeLimit = Number.isFinite(parsedLimit) ? parsedLimit : 8; + const limit = Math.min(Math.max(safeLimit, 1), 50);
…idebar-mobile-dropdown # Conflicts: # apps/web/src/app/dashboard/[driveId]/[pageId]/page.tsx # apps/web/src/components/shared/RecentsDropdown.tsx
1 participant
What changed
Why
Verification
Summary by CodeRabbit
Improvements
New Features