-
Notifications
You must be signed in to change notification settings - Fork 5
fix: (Pictique) refactor layout structure and fix mobile comments #656
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
📝 WalkthroughWalkthroughRefactors Header to derive heading/variant from the current route; adds a reusable MainPanel two-column layout and RightPanel placeholders; moves comment handling into a central comments store with responsive RightPanel/Drawer rendering; updates several protected route layouts to wrap children with MainPanel; adds Drawer onClose and adjusts pane breakpoints. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Home as Home Page
participant Store as Comment Store
participant API as Backend API
participant Right as RightPanel/Drawer
User->>Home: Click "view comments" on post
Home->>Home: set activePostId, open RightPanel or Drawer
Home->>Store: fetchComments(activePostId)
Store->>API: GET /comments/{postId}
API-->>Store: comments[]
Store-->>Right: update $comments
Right-->>User: display comments
User->>Right: submit comment
Right->>Store: createComment(postId, text)
Store->>API: POST /comments
API-->>Store: created comment
Store-->>Right: append comment
Right-->>User: show new comment
sequenceDiagram
participant User
participant Messages as Messages Page
participant Modal
participant Store as Chat Store
participant API as Backend API
User->>Messages: Click "New Chat" or "New Group"
Messages->>Modal: open modal (search & select)
User->>Modal: enter search term
Modal->>Store: queryUsers(term)
Store->>API: GET /users?search=term
API-->>Store: users[]
Store-->>Modal: show results
User->>Modal: select members
alt Direct message (1 member)
Modal->>Store: createChat(memberId)
Store->>API: POST /chats
else Group chat (2+ members)
Modal->>Store: createGroup(name, memberIds)
Store->>API: POST /groups
end
API-->>Store: created chat/group
Store-->>Messages: update chat list
Messages-->>User: show created chat/group
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom Pre-merge checks in the settings. 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 |
…us protected routes; enhance user experience with consistent UI elements and improved comment handling in home and messages sections.
1ea7db8 to
0c6f1b2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
platforms/pictique/src/lib/fragments/Header/Header.svelte (1)
107-125: Update component documentation to reflect the new API.The documentation block still references the old prop-based interface (
variant,heading,callbackprops), but the component now derives these values from the route. Update the docs to reflect that the component no longer accepts these props and instead derives its state from$app/state.platforms/pictique/src/routes/(protected)/messages/+page.svelte (1)
194-204: Fix member filtering logic in group chat creation.Line 196 filters
allMembersto find only the first selected member (selectedMembers[0]), but for group chats, all selected members should be included. This means the group name will only reflect one member's name instead of all members.🔎 Proposed fix
} else { // Create group chat - const groupMembers = allMembers.filter((m) => m.id === selectedMembers[0]); + const groupMembers = allMembers.filter((m) => selectedMembers.includes(m.id)); const groupName = groupMembers.map((m) => m.name ?? m.handle ?? m.ename).join(', '); // Create group chat via API await apiClient.post('/api/chats', { name: groupName, participantIds: selectedMembers, isGroup: true }); }
🤖 Fix all issues with AI Agents
In @platforms/pictique/src/lib/fragments/Header/Header.svelte:
- Around line 18-20: The route matching uses broad route.includes(...) checks
causing false positives (e.g., route.includes('post') matches /posts or
/repost); update the logic where route.includes('/post/audience') and
route.includes('post') are used to use stricter checks—for example use route ===
'/post' or route.startsWith('/post/') for multi-segment matches, or a regex like
/^\/post(\/|$)/ to match the post segment exactly; apply the same stricter
pattern to the other occurrence of route.includes('/post') in this component and
adjust the heading assignments accordingly.
In @platforms/pictique/src/routes/(protected)/+layout.svelte:
- Line 43: The grid column setup on the main element uses
"grid-cols-[20vw_auto]" which lets the content column size to its minimum;
update the class on the <main> element (the element that currently has class
"block h-dvh grid-cols-[20vw_auto] md:grid") to use "grid-cols-[20vw_1fr]" so
the second column uses the remaining space (1fr) and prevents the main content
from collapsing narrower than the viewport.
- Around line 52-54: The current guard uses page.params.id in the parent layout
where page.params.id is undefined, producing "/messages/undefined" and failing
to hide BottomNav; replace the conditional that checks route !==
`/messages/${page.params.id}` with a pathname check such as using
route.startsWith — e.g. render BottomNav only when
!route.startsWith('/messages/') — or alternatively set and read a dedicated
store for the active message ID during navigation and use that store to decide
whether to show BottomNav (referencing the existing route variable and the
BottomNav component).
In @platforms/pictique/src/routes/(protected)/home/+page.svelte:
- Line 241: The h3 elements use an invalid Tailwind class `text-black-600`;
update both occurrences (the <h3 class="text-black-600 mb-6 text-center">
instances) to a valid utility such as `text-black mb-6 text-center` or
`text-gray-600 mb-6 text-center` depending on the intended contrast; replace the
class string in both locations (lines referenced in the review) accordingly.
- Around line 240-278: The <MessageInput> component is placed directly inside a
<ul>, which is invalid HTML; refactor the markup so the <ul> contains only <li>
children and move <MessageInput> out as a sibling by wrapping the header, the
<ul> of <Comment> items and the <MessageInput> in a container element (e.g.,
<div class="pb-4">) — keep the existing <h3>, the {#each $comments as comment
(comment.id)} producing <li><Comment ... /></li> intact, then place
<MessageInput class="sticky start-0 bottom-4 mt-4 w-full px-2" variant="comment"
src={profile?.avatarUrl ?? '/images/user.png'} bind:value={commentValue}
{handleSend} bind:input={commentInput} /> after the </ul>.
- Around line 89-102: The effect fetching comments can race when
showComments.value and $activePostId change rapidly; capture the id at fetch
start (e.g., const currentId = $activePostId) or use an AbortController for
fetchComments, then before assigning results or setting
isCommentsLoading/commentsError verify the active id still matches currentId (or
that the request wasn’t aborted); update the logic around fetchComments,
isCommentsLoading, and commentsError to only apply results when the guard passes
or on non-aborted completion to prevent stale comments from overwriting newer
ones.
In @platforms/pictique/src/routes/(protected)/messages/+page.svelte:
- Line 282: The unread prop is being passed inverted to the Message component
(unread={!message.unread}); update the call site so it forwards the actual
boolean from the message model (pass unread={message.unread}) unless the Message
component explicitly expects inverted semantics—locate the Message usage in
+page.svelte and replace the negation with the direct property, and if needed,
confirm Message's prop handling to avoid semantic mismatch.
🧹 Nitpick comments (7)
platforms/pictique/src/lib/fragments/Header/Header.svelte (1)
11-29: Prefer$derivedover$effect+$statefor heading.The
headingvariable is computed fromroute, making it a perfect candidate for$derivedinstead of$state+$effect. This simplifies the reactivity model and aligns with Svelte 5 best practices.🔎 Proposed refactor using $derived
- let heading = $state(''); - - $effect(() => { - if (route.includes('home')) { - heading = 'Feed'; - } else if (route.includes('discover')) { - heading = 'Search'; - } else if (route.includes('/post/audience')) { - heading = 'Audience'; - } else if (route.includes('post')) { - heading = 'Upload photo'; - } else if (route === '/messages') { - heading = 'Messages'; - } else if (route.includes('settings')) { - heading = 'Settings'; - } else if (route.includes('profile')) { - heading = 'Profile'; - } - }); + let heading = $derived.by(() => { + if (route.includes('home')) { + return 'Feed'; + } else if (route.includes('discover')) { + return 'Search'; + } else if (route.includes('/post/audience')) { + return 'Audience'; + } else if (route.includes('post')) { + return 'Upload photo'; + } else if (route === '/messages') { + return 'Messages'; + } else if (route.includes('settings')) { + return 'Settings'; + } else if (route.includes('profile')) { + return 'Profile'; + } + return ''; + });platforms/pictique/src/routes/(protected)/messages/+page.svelte (2)
318-426: Consider extracting modal UI into reusable components.Both the "Start a New Chat" modal (lines 318-426) and "Create New Group" modal (lines 428-601) contain significant duplicate markup, including:
- Modal wrapper and backdrop styling
- Close button with inline SVG
- Form structure and action buttons
Extracting these into reusable
ModalandModalHeadercomponents would reduce duplication, improve maintainability, and make the page code more readable.Also applies to: 428-601
216-218: Consider replacingalert()with a toast notification system.Using
alert()for error messages (lines 218, 249) provides poor user experience as it blocks interaction and looks dated. Consider implementing a toast/notification component for non-blocking error feedback.Also applies to: 247-250
platforms/pictique/src/routes/(protected)/post/+layout.svelte (1)
11-13: Consider omitting unusedRightPanelsnippet.The
RightPanelsnippet is empty and renders nothing. If there are no immediate plans to add content here, consider removing it for clarity. Empty snippets can be added back when needed.If this is a placeholder for future functionality, consider adding a comment to document the intent.
Alternative without empty snippet
<MainPanel ><div class="flex flex-col gap-5"> {@render children()} </div> - {#snippet RightPanel()} - <div></div> - {/snippet} </MainPanel>platforms/pictique/src/lib/fragments/MainPanel/MainPanel.svelte (1)
23-30: Remove redundant optional chaining in snippet render.Line 27 uses
RightPanel?.()inside an#if RightPanelblock where RightPanel's existence is already guaranteed. The optional chaining operator is unnecessary here.🔎 Proposed fix
{#if RightPanel} <aside class="hide-scrollbar relative hidden h-dvh w-[30vw] overflow-y-scroll border border-y-0 border-e-0 border-s-gray-200 px-8 pt-12 md:flex md:flex-col" > - {@render RightPanel?.()} + {@render RightPanel()} </aside> {/if}platforms/pictique/src/lib/fragments/RightAside/RightAside.svelte (1)
11-22: Minor: Redundant optional chaining in header render.On line 17,
header?.()uses optional chaining, but the{#if header}guard on line 15 already ensuresheaderis defined. The?.is harmless but unnecessary.🔎 Suggested simplification
{#if header} <h2 class="mb-10 text-lg font-semibold"> - {@render header?.()} + {@render header()} </h2> {/if}platforms/pictique/src/routes/(protected)/home/+page.svelte (1)
234-280: Duplicated comment rendering logic between desktop and mobile.The comment list rendering (lines 249-268 and 291-310) and the
MessageInputplacement are nearly identical between the desktopRightPanelsnippet and the mobileDrawer. This violates DRY and increases maintenance burden.Consider extracting a shared
CommentsListsnippet or component to render comments and input in both contexts.🔎 Example extraction
{#snippet CommentsList(containerClass: string, inputClass: string)} {#if isCommentsLoading} <li class="text-center text-gray-500">Loading comments...</li> {:else if commentsError} <li class="text-center text-red-500">{commentsError}</li> {:else} {#each $comments as comment (comment.id)} <li class="mb-4"> <Comment comment={{ userImgSrc: comment.author.avatarUrl, name: comment.author.name || comment.author.handle, commentId: comment.id, comment: comment.text, isUpVoted: false, isDownVoted: false, upVotes: 0, time: new Date(comment.createdAt).toLocaleDateString(), replies: [] }} handleReply={() => { commentInput?.focus(); }} /> </li> {/each} {/if} <MessageInput class={inputClass} variant="comment" src={profile?.avatarUrl ?? '/images/user.png'} bind:value={commentValue} {handleSend} bind:input={commentInput} /> {/snippet}Then use
{@render CommentsList("sticky ...", "fixed ...")}in both places.Also applies to: 283-321
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
platforms/pictique/src/lib/fragments/Header/Header.svelteplatforms/pictique/src/lib/fragments/MainPanel/MainPanel.svelteplatforms/pictique/src/lib/fragments/RightAside/RightAside.svelteplatforms/pictique/src/routes/(protected)/+layout.svelteplatforms/pictique/src/routes/(protected)/discover/+page.svelteplatforms/pictique/src/routes/(protected)/home/+page.svelteplatforms/pictique/src/routes/(protected)/messages/+layout.svelteplatforms/pictique/src/routes/(protected)/messages/+page.svelteplatforms/pictique/src/routes/(protected)/post/+layout.svelteplatforms/pictique/src/routes/(protected)/profile/+layout.svelteplatforms/pictique/src/routes/(protected)/settings/+layout.svelteplatforms/pictique/src/routes/+layout.svelte
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2026-01-05T11:46:43.214Z
Learnt from: Bekiboo
Repo: MetaState-Prototype-Project/prototype PR: 653
File: platforms/pictique/src/routes/(auth)/auth/+page.svelte:173-173
Timestamp: 2026-01-05T11:46:43.214Z
Learning: In Tailwind CSS v4, gradient utilities were renamed from bg-gradient-to-{direction} to bg-linear-to-{direction}. For all Svelte components (e.g., any .svelte files in the project), replace usages like bg-gradient-to-r, bg-gradient-to-l, etc. with bg-linear-to-r, bg-linear-to-l, etc. Ensure consistency across the codebase, update any documentation or design tokens, and run visual/test checks to confirm gradients render as intended.
Applied to files:
platforms/pictique/src/routes/+layout.svelteplatforms/pictique/src/lib/fragments/MainPanel/MainPanel.svelteplatforms/pictique/src/lib/fragments/RightAside/RightAside.svelteplatforms/pictique/src/routes/(protected)/+layout.svelteplatforms/pictique/src/lib/fragments/Header/Header.svelteplatforms/pictique/src/routes/(protected)/post/+layout.svelteplatforms/pictique/src/routes/(protected)/messages/+layout.svelteplatforms/pictique/src/routes/(protected)/messages/+page.svelteplatforms/pictique/src/routes/(protected)/profile/+layout.svelteplatforms/pictique/src/routes/(protected)/discover/+page.svelteplatforms/pictique/src/routes/(protected)/home/+page.svelteplatforms/pictique/src/routes/(protected)/settings/+layout.svelte
🔇 Additional comments (13)
platforms/pictique/src/routes/+layout.svelte (1)
46-46: LGTM: Correct Tailwind v4 utility usage.The change from
h-[100dvh]toh-dvhcorrectly adopts Tailwind v4's native support for dynamic viewport height units. This is cleaner and more semantic than the arbitrary value syntax.Also applies to: 50-50
platforms/pictique/src/routes/(protected)/profile/+layout.svelte (1)
1-9: LGTM: Clean MainPanel wrapper implementation.This layout correctly follows the MainPanel composition pattern established across the PR. The implementation is simple and consistent with other protected route layouts.
platforms/pictique/src/routes/(protected)/settings/+layout.svelte (1)
1-14: LGTM! Clean refactoring to MainPanel pattern.The layout correctly wraps children inside MainPanel with a flex container and provides an empty RightPanel placeholder for future use. This aligns with the consistent two-column layout pattern introduced across protected routes.
platforms/pictique/src/routes/(protected)/messages/+layout.svelte (1)
1-12: LGTM! Clean and minimal MainPanel integration.The messages layout correctly adopts the MainPanel pattern with direct children rendering (no extra wrapper div). This is appropriate and consistent with the broader refactoring.
platforms/pictique/src/routes/(protected)/discover/+page.svelte (1)
1-81: LGTM! Clean structural refactoring with MainPanel.The discover page correctly integrates the MainPanel wrapper while preserving all existing functionality. The search, display, and interaction logic remains unchanged—this is purely a layout improvement.
platforms/pictique/src/lib/fragments/MainPanel/MainPanel.svelte (2)
1-12: LGTM! Well-structured component interface.The TypeScript interface correctly defines the MainPanel props with a required
childrensnippet and an optionalRightPanelsnippet, extending HTML div attributes. This follows Svelte 5 best practices for snippet-based composition.
14-22: LGTM! Main panel structure correctly implements the overflow fix.The
min-w-0class on line 16 correctly prevents flex child overflow issues (the "sidebar crushing" bug mentioned in the PR description). The layout structure with Header and children rendering is sound.platforms/pictique/src/routes/(protected)/+layout.svelte (2)
22-40: LGTM! fetchProfile correctly assigns ownerId.The addition of
ownerId = getAuthId()at line 23 properly initializes the owner ID before the profile fetch. The auth guard logic and error handling remain intact.
57-101: LGTM! Disclaimer modal unchanged except for minor formatting.The CreatePostModal and disclaimer modal logic remain intact. The only change is a minor text line break adjustment at lines 76-77, which improves readability without affecting functionality.
platforms/pictique/src/lib/fragments/RightAside/RightAside.svelte (1)
5-8: LGTM on the props refactoring.The shift from
asideContentto the implicitchildrensnippet is cleaner and aligns with Svelte 5 conventions. The optionalheaderprop is appropriately typed.platforms/pictique/src/routes/(protected)/home/+page.svelte (3)
60-68: LGTM on simplifiedhandleSend.The function properly validates inputs before calling
createCommentand clears the input on success. Error handling with console logging is appropriate for this context.
111-232: LGTM on the posts list and infinite scroll integration.The optimistic like updates with rollback on error, sentinel-based infinite scroll, and proper loading/error states are well implemented.
234-280: No action needed—RightPanel is correctly wired to MainPanel.The
RightPanelsnippet is properly passed toMainPanelvia Svelte's snippet mechanism.MainPanelaccepts it as an optionalRightPanelprop and conditionally renders it in the desktop sidebar (lines 23–29 of MainPanel.svelte).
| avatar={message.avatar} | ||
| username={message.name ?? message.username} | ||
| text={message.text} | ||
| unread={!message.unread} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix inverted unread prop logic.
The unread prop is receiving the inverted value: unread={!message.unread}. This means:
- When
message.unreadistrue(message IS unread), the component receivesfalse - When
message.unreadisfalse(message IS read), the component receivestrue
This appears to be a logic error unless the Message component expects inverted semantics.
🔎 Proposed fix
<Message
class="mb-2"
avatar={message.avatar}
username={message.name ?? message.username}
text={message.text}
- unread={!message.unread}
+ unread={message.unread}
callback={() => {
heading.set(message.username);
goto(`/messages/${message.id}`);
}}
/>📝 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.
| unread={!message.unread} | |
| <Message | |
| class="mb-2" | |
| avatar={message.avatar} | |
| username={message.name ?? message.username} | |
| text={message.text} | |
| unread={message.unread} | |
| callback={() => { | |
| heading.set(message.username); | |
| goto(`/messages/${message.id}`); | |
| }} | |
| /> |
🤖 Prompt for AI Agents
In @platforms/pictique/src/routes/(protected)/messages/+page.svelte at line 282,
The unread prop is being passed inverted to the Message component
(unread={!message.unread}); update the call site so it forwards the actual
boolean from the message model (pass unread={message.unread}) unless the Message
component explicitly expects inverted semantics—locate the Message usage in
+page.svelte and replace the negation with the direct property, and if needed,
confirm Message's prop handling to avoid semantic mismatch.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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)
platforms/pictique/src/routes/(protected)/+layout.svelte (1)
22-28: Add null safety check forownerIdbefore making the API call.If
getAuthId()returnsnull, the API request on line 28 will attempt to fetch/api/users/null, which will likely fail. Consider checking ifownerIdis valid before proceeding with the API call.🔎 Proposed fix
async function fetchProfile() { ownerId = getAuthId(); + if (!ownerId || !getAuthToken()) { + goto('/auth'); + return; + } try { - if (!getAuthToken()) { - goto('/auth'); - } const response = await apiClient.get(`/api/users/${ownerId}`).catch((e: AxiosError) => { if (e.response?.status === 401) { goto('/auth'); } });
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
platforms/pictique/src/lib/fragments/Header/Header.svelteplatforms/pictique/src/routes/(protected)/+layout.svelte
🚧 Files skipped from review as they are similar to previous changes (1)
- platforms/pictique/src/lib/fragments/Header/Header.svelte
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2026-01-05T11:46:43.214Z
Learnt from: Bekiboo
Repo: MetaState-Prototype-Project/prototype PR: 653
File: platforms/pictique/src/routes/(auth)/auth/+page.svelte:173-173
Timestamp: 2026-01-05T11:46:43.214Z
Learning: In Tailwind CSS v4, gradient utilities were renamed from bg-gradient-to-{direction} to bg-linear-to-{direction}. For all Svelte components (e.g., any .svelte files in the project), replace usages like bg-gradient-to-r, bg-gradient-to-l, etc. with bg-linear-to-r, bg-linear-to-l, etc. Ensure consistency across the codebase, update any documentation or design tokens, and run visual/test checks to confirm gradients render as intended.
Applied to files:
platforms/pictique/src/routes/(protected)/+layout.svelte
⏰ 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). (2)
- GitHub Check: lint
- GitHub Check: build
🔇 Additional comments (1)
platforms/pictique/src/routes/(protected)/+layout.svelte (1)
43-43: Grid layout fix correctly applied.The grid column definition now uses
1frfor the second column, which ensures the main content area fills all available space. This addresses the previous review concern about the content column sizing to its minimum width.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
platforms/pictique/src/lib/fragments/Drawer/Drawer.svelte (1)
45-67: Add cleanup in onDestroy to prevent resource leaks.The
CupertinoPaneinstance is created inonMountbut never cleaned up. If the component is unmounted (e.g., through navigation or conditional rendering), the drawer instance may persist, leading to:
- Memory leaks from unremoved event listeners
- Orphaned DOM elements (backdrop, pane container)
- Zombie UI elements that remain visible after unmount
🔎 Proposed fix
Add cleanup logic using Svelte's
onDestroy:-import { onMount, type Snippet } from 'svelte'; +import { onMount, onDestroy, type Snippet } from 'svelte'; onMount(() => { if (!drawerElement) return; drawer = new CupertinoPane(drawerElement, { // ... config }); }); + +onDestroy(() => { + if (drawer) { + drawer.destroy({ animate: false }); + } +});Note: Use
animate: falsein cleanup to avoid animation delays during unmount.platforms/pictique/src/routes/(protected)/+layout.svelte (1)
22-38: Add return statement after auth redirect to prevent unnecessary API call.On line 26, after calling
goto('/auth'), execution continues because there's no return statement. This causes the API call on line 28 to execute even though the user is being navigated away.🔎 Proposed fix
try { if (!getAuthToken()) { goto('/auth'); + return; } const response = await apiClient.get(`/api/users/${ownerId}`).catch((e: AxiosError) => {Additionally, the nested error handling pattern (inner
.catch()on line 28 and outertry-catchon lines 24-37) could be simplified for clarity, though the current implementation works correctly.🔎 Optional refactor for clearer error handling
async function fetchProfile() { ownerId = getAuthId(); + if (!getAuthToken()) { + goto('/auth'); + return; + } + try { - if (!getAuthToken()) { - goto('/auth'); - } - const response = await apiClient.get(`/api/users/${ownerId}`).catch((e: AxiosError) => { - if (e.response?.status === 401) { - goto('/auth'); - } - }); - if (!response) return; + const response = await apiClient.get(`/api/users/${ownerId}`); profile = response.data; - } catch (err) { - console.log(err instanceof Error ? err.message : 'Failed to load profile'); + } catch (err: unknown) { + if (err instanceof Error && 'response' in err) { + const axiosError = err as AxiosError; + if (axiosError.response?.status === 401) { + goto('/auth'); + return; + } + } + console.log(err instanceof Error ? err.message : 'Failed to load profile'); } }This approach separates auth checks from API error handling, making the flow more explicit.
🤖 Fix all issues with AI Agents
In @platforms/pictique/src/lib/fragments/Drawer/Drawer.svelte:
- Around line 24-32: The dismiss() and downSwipeHandler() functions currently
call onClose?.() directly after invoking drawer.destroy(), which can result in
onClose being called twice because CupertinoPane's destroy() triggers the
onWillDismiss event that also invokes onClose; remove the direct onClose?.()
calls from dismiss() and downSwipeHandler() so the onWillDismiss handler is the
single source of truth for closing behavior (or, if you must keep a safeguard,
introduce a simple boolean guard like hasClosed used by onWillDismiss, dismiss,
and downSwipeHandler to ensure onClose is only invoked once).
In @platforms/pictique/src/routes/(protected)/home/+page.svelte:
- Around line 248-251: Null-safety: guard against missing comment.author when
rendering the Comment component by using optional chaining and sensible
fallbacks for avatar and name/handle—e.g., replace direct accesses
comment.author.avatarUrl and comment.author.name || comment.author.handle with
checks like comment.author?.avatarUrl and (comment.author?.name ||
comment.author?.handle || 'Unknown') in the Comment prop object; apply the same
changes to the duplicate usage in the mobile drawer section (the Comment
invocation around lines 298-299) so rendering won't throw when author is absent.
- Around line 90-103: The effect that calls fetchComments when
showComments.value and $activePostId change can apply out‑of‑order responses;
fix by making the effect track and cancel/outdate previous requests: generate a
local requestId (or use an AbortController) each time inside the $effect before
calling fetchComments($activePostId), pass the signal or capture the id with the
promise, and only set commentsError, isCommentsLoading and store the returned
comments if the id/signal still matches (or not aborted). Also ensure any
previous controller is aborted (or previous requestId invalidated) when starting
a new fetch so stale responses cannot overwrite newer state.
🧹 Nitpick comments (3)
platforms/pictique/src/lib/fragments/Drawer/Drawer.svelte (1)
59-60: Consider handling window resize for break heights.The drawer break heights are calculated using
window.innerHeightat mount time and remain fixed. If the user resizes the browser window (e.g., device rotation, desktop window resize), the drawer dimensions won't adapt, potentially causing suboptimal UX.💡 Optional enhancement
If supporting dynamic resizing is valuable for your use case, consider updating break heights on window resize:
+import { onMount, onDestroy, type Snippet } from 'svelte'; +function updateBreaks() { + if (drawer) { + drawer.setBreakpoints({ + top: { enabled: true, height: window.innerHeight * 0.9 }, + middle: { enabled: true, height: window.innerHeight * 0.5 } + }); + } +} + onMount(() => { if (!drawerElement) return; drawer = new CupertinoPane(drawerElement, { // ... existing config }); + window.addEventListener('resize', updateBreaks); }); + +onDestroy(() => { + window.removeEventListener('resize', updateBreaks); + if (drawer) { + drawer.destroy({ animate: false }); + } +});Note: Verify that CupertinoPane v1.4.22 supports
setBreakpoints()or equivalent API.platforms/pictique/src/routes/(protected)/home/+page.svelte (2)
61-70: Consider showing error feedback to users.The
handleSendfunction logs comment creation errors to the console but doesn't provide user-visible feedback. Users won't know if their comment failed to post.🔎 Suggested enhancement to show error state
Add a state variable for comment submission errors and display it in the UI:
let isCommentsLoading = $state(false); let commentsError = $state<string | null>(null); +let commentSubmitError = $state<string | null>(null); let isDrawerOpen = $state(false);const handleSend = async () => { if (!$activePostId || !commentValue.trim()) return; + commentSubmitError = null; try { await createComment($activePostId, commentValue); commentValue = ''; } catch (err) { - console.error('Failed to create comment:', err); + commentSubmitError = err instanceof Error ? err.message : 'Failed to post comment'; + console.error('Failed to create comment:', err); } };Then display the error near the MessageInput component.
246-264: Consider extracting duplicated comment rendering logic.The Comment rendering logic is duplicated between the desktop RightPanel (lines 246-264) and mobile Drawer (lines 294-312). Extracting this into a reusable snippet would reduce duplication and make maintenance easier.
🔎 Suggested refactor using a snippet
+{#snippet CommentList()} + {#each $comments as comment (comment.id)} + <li class="mb-4"> + <Comment + comment={{ + userImgSrc: comment.author?.avatarUrl ?? '/images/user.png', + name: comment.author?.name || comment.author?.handle || 'Unknown User', + commentId: comment.id, + comment: comment.text, + isUpVoted: false, + isDownVoted: false, + upVotes: 0, + time: new Date(comment.createdAt).toLocaleDateString(), + replies: [] + }} + handleReply={() => { + commentInput?.focus(); + }} + /> + </li> + {/each} +{/snippet}Then use
{@render CommentList()}in both the RightPanel and Drawer sections.Also applies to: 294-312
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
platforms/pictique/src/lib/fragments/Drawer/Drawer.svelteplatforms/pictique/src/routes/(protected)/+layout.svelteplatforms/pictique/src/routes/(protected)/home/+page.svelte
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2026-01-05T11:46:43.214Z
Learnt from: Bekiboo
Repo: MetaState-Prototype-Project/prototype PR: 653
File: platforms/pictique/src/routes/(auth)/auth/+page.svelte:173-173
Timestamp: 2026-01-05T11:46:43.214Z
Learning: In Tailwind CSS v4, gradient utilities were renamed from bg-gradient-to-{direction} to bg-linear-to-{direction}. For all Svelte components (e.g., any .svelte files in the project), replace usages like bg-gradient-to-r, bg-gradient-to-l, etc. with bg-linear-to-r, bg-linear-to-l, etc. Ensure consistency across the codebase, update any documentation or design tokens, and run visual/test checks to confirm gradients render as intended.
Applied to files:
platforms/pictique/src/routes/(protected)/+layout.svelteplatforms/pictique/src/lib/fragments/Drawer/Drawer.svelteplatforms/pictique/src/routes/(protected)/home/+page.svelte
📚 Learning: 2025-08-08T04:15:35.098Z
Learnt from: sosweetham
Repo: MetaState-Prototype-Project/prototype PR: 279
File: infrastructure/control-panel/src/app.css:0-0
Timestamp: 2025-08-08T04:15:35.098Z
Learning: The MetaState-Prototype-Project uses Tailwind CSS v3, where the theme directive is valid and supported. The theme block should not be changed to :root or layer base in this project.
Applied to files:
platforms/pictique/src/routes/(protected)/home/+page.svelte
⏰ 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: build
🔇 Additional comments (3)
platforms/pictique/src/lib/fragments/Drawer/Drawer.svelte (1)
57-61: Nice UX improvements for mobile drawer behavior.The changes to drawer configuration improve the mobile experience:
- Starting at
'top'(90% height) provides better initial content visibility, which aligns with the PR's goal of fixing mobile comment display.- Two break points (
topandmiddle) give users control over drawer height.- Consistent backdrop tap dismissal follows standard mobile UX patterns.
These changes effectively support the PR objectives for mobile comment functionality.
Also applies to: 63-63
platforms/pictique/src/routes/(protected)/+layout.svelte (2)
43-55: LGTM! Layout refactoring correctly addresses previous review feedback.The changes successfully fix the issues identified in previous reviews:
- Line 43: Grid columns now use
1frinstead ofauto, ensuring the main content fills available space- Line 52: Route guard now uses regex pattern matching instead of the undefined
page.params.id, correctly hiding BottomNav on message detail pagesThe Svelte 5 snippet syntax
{@render children()}on line 50 is also correct for the new architecture.
57-101: Modal implementations are correct.The CreatePostModal and Disclaimer modal are properly implemented with correct state bindings and close handlers. The disclaimer enforcement (redirecting to /auth if not confirmed) provides good UX protection for the prototype.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @platforms/pictique/src/routes/(protected)/home/+page.svelte:
- Line 277: The H3 element rendering the comment count uses an invalid Tailwind
class "text-black-600"; update the class on the <h3> (the element containing
"{$comments.length} Comments") to use a valid Tailwind color utility such as
"text-black" or "text-gray-600" (e.g., replace "text-black-600" with your chosen
valid class) so the styling compiles correctly.
- Line 242: The class on the H3 element uses the invalid Tailwind utility
`text-black-600`; update the class on the H3 (the element that currently has
class "text-black-600 mb-6 text-center") to a valid utility such as `text-black
mb-6 text-center` or `text-gray-600 mb-6 text-center` depending on the intended
color.
🧹 Nitpick comments (1)
platforms/pictique/src/lib/ui/Avatar/Avatar.svelte (1)
33-33: Remove debug statement before merging.The
$inspect(img)statement is a development debugging aid that should be removed before merging to main.🔎 Suggested fix
- $inspect(img);
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
platforms/pictique/src/lib/fragments/Drawer/Drawer.svelteplatforms/pictique/src/lib/ui/Avatar/Avatar.svelteplatforms/pictique/src/routes/(protected)/home/+page.svelte
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2026-01-05T11:46:43.214Z
Learnt from: Bekiboo
Repo: MetaState-Prototype-Project/prototype PR: 653
File: platforms/pictique/src/routes/(auth)/auth/+page.svelte:173-173
Timestamp: 2026-01-05T11:46:43.214Z
Learning: In Tailwind CSS v4, gradient utilities were renamed from bg-gradient-to-{direction} to bg-linear-to-{direction}. For all Svelte components (e.g., any .svelte files in the project), replace usages like bg-gradient-to-r, bg-gradient-to-l, etc. with bg-linear-to-r, bg-linear-to-l, etc. Ensure consistency across the codebase, update any documentation or design tokens, and run visual/test checks to confirm gradients render as intended.
Applied to files:
platforms/pictique/src/lib/ui/Avatar/Avatar.svelteplatforms/pictique/src/lib/fragments/Drawer/Drawer.svelteplatforms/pictique/src/routes/(protected)/home/+page.svelte
📚 Learning: 2025-08-08T04:15:35.098Z
Learnt from: sosweetham
Repo: MetaState-Prototype-Project/prototype PR: 279
File: infrastructure/control-panel/src/app.css:0-0
Timestamp: 2025-08-08T04:15:35.098Z
Learning: The MetaState-Prototype-Project uses Tailwind CSS v3, where the theme directive is valid and supported. The theme block should not be changed to :root or layer base in this project.
Applied to files:
platforms/pictique/src/routes/(protected)/home/+page.svelte
⏰ 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). (2)
- GitHub Check: build
- GitHub Check: lint
🔇 Additional comments (8)
platforms/pictique/src/lib/ui/Avatar/Avatar.svelte (1)
24-24: LGTM! Layout stability improvements.The addition of
shrink-0,aspect-square, andobject-coverclasses enhances the avatar's layout behavior in flex contexts, preventing unwanted crushing and ensuring consistent rendering—directly addressing the PR's layout refactoring objectives.platforms/pictique/src/lib/fragments/Drawer/Drawer.svelte (3)
12-20: LGTM: onClose callback prop added correctly.The optional onClose callback is properly typed and destructured.
55-62: LGTM: Drawer breaks and event wiring configured correctly.The initialBreak set to 'top' and breaks configuration with responsive heights are appropriate. The onClose callback is now only triggered via onWillDismiss, properly addressing the previous double-invocation concern.
Note: The breaks heights use
window.innerHeightat mount time and won't update on window resize. This is typically acceptable for drawer use cases.
77-83: LGTM: Scrolling styles configured appropriately.The explicit
overflow-y: scrollensures consistent behavior, and scrollbar hiding maintains a clean mobile drawer appearance.platforms/pictique/src/routes/(protected)/home/+page.svelte (4)
5-7: LGTM: Imports and state variables properly updated.The additions of MainPanel, comment store functions, and state variables (isCommentsLoading, commentsError, isDrawerOpen) properly support the new comment handling flow.
Also applies to: 19-19, 29-31
61-70: LGTM: Comment submission logic is clean and correct.The handleSend function properly validates input, calls the createComment API, and includes appropriate error handling.
90-108: LGTM: Race condition properly mitigated.The $effect correctly captures the targetPostId and validates it matches $activePostId before updating state. This ensures that rapid post selection won't cause stale comments to overwrite newer ones, addressing the previous review concern.
309-335: LGTM: Comment rendering with proper null safety.The SingleComment snippet correctly uses optional chaining and fallback values for comment author data, addressing the previous null safety concern.
Description of change
Issue Number
Closes #626
Type of change
How the change has been tested
On Windows, Chrome, both desktop and mobile views
Change checklist
Summary by CodeRabbit
New Features
Refactor
✏️ Tip: You can customize this high-level summary in your review settings.