fix: use cached tab title and pageType in mobile RecentsDropdown#375
fix: use cached tab title and pageType in mobile RecentsDropdown#3752witstudios merged 1 commit intomasterfrom
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
📝 WalkthroughWalkthroughReplaced local tab-store recents with SWR server fetching; added loading/empty states; adjusted RecentsSection empty handling; changed dashboard layout default to render children; middleware now accepts session bearer tokens prefixed with Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 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 |
40b1436 to
ac4d7b7
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@apps/web/src/components/shared/RecentsDropdown.tsx`:
- Around line 22-25: The fetcher function should be given an explicit return
type to avoid implicit any from response.json(); update the fetcher signature
(fetcher) to return Promise<{ recents: RecentPage[] }> and ensure the returned
value is typed (e.g., await response.json() cast or typed as { recents:
RecentPage[] }) so it matches the SWR generic parameter and satisfies
TypeScript.
🧹 Nitpick comments (1)
apps/web/src/components/shared/RecentsDropdown.tsx (1)
35-41: Add SWR editing-protection pause for consistency with similar components.This component should follow the established pattern used in
UserDropdownand other navigation/data-fetching components. AddisPaused: () => hasLoadedRef.current && isEditingActive()to pause refreshes during editing, which prevents interruptions when users are actively editing pages elsewhere in the app. This requires importinguseEditingStoreand tracking load state with auseRef.
| const fetcher = async (url: string) => { | ||
| const response = await fetchWithAuth(url); | ||
| if (!response.ok) throw new Error('Failed to fetch'); | ||
| return response.json(); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, check the file exists and get its content
wc -l apps/web/src/components/shared/RecentsDropdown.tsxRepository: 2witstudios/PageSpace
Length of output: 118
🏁 Script executed:
# Read the file to see the actual code at lines 22-25 and surrounding context
head -50 apps/web/src/components/shared/RecentsDropdown.tsx | cat -nRepository: 2witstudios/PageSpace
Length of output: 1829
🏁 Script executed:
# Search for RecentPage type definition in the codebase
rg "type RecentPage|interface RecentPage" -t ts -t tsxRepository: 2witstudios/PageSpace
Length of output: 92
🏁 Script executed:
# Check for useSWR usage patterns in the file
rg "useSWR" apps/web/src/components/shared/RecentsDropdown.tsx -A 5 -B 2Repository: 2witstudios/PageSpace
Length of output: 551
🏁 Script executed:
# Check the API route to confirm the response structure
head -50 apps/web/src/app/api/user/recents/route.ts | cat -nRepository: 2witstudios/PageSpace
Length of output: 1838
🏁 Script executed:
# Also search for RecentPage type to verify the imported type
rg "export.*type RecentPage|export.*interface RecentPage" -A 3Repository: 2witstudios/PageSpace
Length of output: 310
Type the SWR fetcher to avoid implicit any from response.json().
The fetcher function lacks an explicit return type. response.json() returns any, which violates the TypeScript guideline. Add an explicit return type matching the SWR generic parameter { recents: RecentPage[] }.
🔧 Proposed fix
-const fetcher = async (url: string) => {
+const fetcher = async (url: string): Promise<{ recents: RecentPage[] }> => {
const response = await fetchWithAuth(url);
if (!response.ok) throw new Error('Failed to fetch');
return response.json();
};📝 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.
| const fetcher = async (url: string) => { | |
| const response = await fetchWithAuth(url); | |
| if (!response.ok) throw new Error('Failed to fetch'); | |
| return response.json(); | |
| const fetcher = async (url: string): Promise<{ recents: RecentPage[] }> => { | |
| const response = await fetchWithAuth(url); | |
| if (!response.ok) throw new Error('Failed to fetch'); | |
| return response.json(); | |
| }; |
🤖 Prompt for AI Agents
In `@apps/web/src/components/shared/RecentsDropdown.tsx` around lines 22 - 25, The
fetcher function should be given an explicit return type to avoid implicit any
from response.json(); update the fetcher signature (fetcher) to return Promise<{
recents: RecentPage[] }> and ensure the returned value is typed (e.g., await
response.json() cast or typed as { recents: RecentPage[] }) so it matches the
SWR generic parameter and satisfies TypeScript.
ac4d7b7 to
f76ca16
Compare
ROOT CAUSE: The dashboard layout returned `<Layout />` without `{children}`
for page routes, which meant the page.tsx component at [driveId]/[pageId]
never mounted. Since the view recording useEffect was in that component,
page views were never recorded to the database.
Fix: Always render `{children}` so the page component mounts and its
effects run, even though it returns null (visual content is in CenterPanel).
Also:
- Allow Bearer session tokens in middleware (for mobile API access)
- RecentsDropdown uses /api/user/recents API instead of tabs store
- RecentsSection shows "No recent pages" instead of hiding when empty
This fixes:
- Page views not being recorded
- Recents not showing in sidebar or mobile dropdown
- Blue dot not clearing when viewing pages
https://claude.ai/code/session_01RPLsmsjvjB7iEd8RgNJUrF
f76ca16 to
01f46de
Compare
The RecentsDropdown (mobile) was showing generic "Page" or "Drive" labels
instead of actual page titles because getTabDisplayInfo() wasn't using
the cached title and pageType properties from the tab object. Now it
properly displays the page names and correct icons.
https://claude.ai/code/session_01RPLsmsjvjB7iEd8RgNJUrF
Summary by CodeRabbit
New Features
Refactor
Bug Fixes