Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughIntroduces client-side image upload handling with per-file and total size validation, UI changes for image previews/removal, submission flow updates, message input submission state, profile picture validation/save handling, and small styling/layout adjustments across pictique components. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant UI as CreatePostModal
participant Validator as validateFileSize
participant Reader as FileReader
participant Store as posts.createPost (API)
User->>UI: selects file(s)
UI->>Validator: validateFileSize(file, maxFile, currentTotal, maxTotal)
alt validation fails
Validator-->>UI: { valid: false, error }
UI-->>User: show error banner
else validation passes
UI->>Reader: readAsDataURL(file)
Reader-->>UI: dataUrl
UI-->>UI: append uploadItems { file, dataUrl }
UI->>User: show preview & update progress
User->>UI: clicks Post
UI->>Store: createPost({ text, images: uploadItems.map(url) })
Store-->>UI: success / error
UI-->>User: reset on success or show submission error
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 inconclusive)
✅ Passed checks (3 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: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
platforms/pictique/src/routes/(protected)/messages/[id]/+page.svelte (2)
103-103:⚠️ Potential issue | 🔴 CriticalFix inverted
isOwnlogic on line 103.
isOwnis set totruewhensender.id !== userId, meaning it'struefor messages from other users. However, theChatMessagecomponent usesisOwnto determine message styling and positioning—whenisOwnistrue, it renders the message with grey background, black text, and positioned on the right (own message formatting). This causes all messages to render on the wrong side.Change line 103 to:
const isOwn = sender.id === userId;
220-220:⚠️ Potential issue | 🟡 MinorRemove unused
srcprop or implement avatar display for direct messages.The
srcprop with the hardcodedpicsum.photosURL is not rendered whenvariant="dm". TheMessageInputcomponent only displays the Avatar whenvariant="comment"(lines 48–50 of MessageInput.svelte). Either remove the unused prop, or modifyMessageInputto display receiver avatars for direct messages and pass the actual participant avatar from the availablechat.participantsdata.platforms/pictique/src/routes/(protected)/settings/account/username/+page.svelte (1)
131-131:⚠️ Potential issue | 🟡 MinorDouble space in placeholder text.
"Edit your public name"has two spaces between "Edit" and "your".Fix
- <Input type="text" placeholder="Edit your public name" bind:value={name} /> + <Input type="text" placeholder="Edit your public name" bind:value={name} />platforms/pictique/src/lib/fragments/CreatePostModal/CreatePostModal.svelte (1)
53-53:⚠️ Potential issue | 🟡 MinorRemove
console.logdebug artifact.This logs the full base64 data URL for every uploaded image. It clutters the console and leaks image data.
Fix
reader.onload = (e) => { const result = e.target?.result; if (typeof result === 'string') { - console.log(result); images = [...images, result]; } };platforms/pictique/src/routes/(protected)/post/+page.svelte (2)
3-3:⚠️ Potential issue | 🟡 Minor
UploadedPostViewimport is unused.The template now renders an inline grid instead of using this component. Remove the dead import.
Fix
- import { SettingsTile, UploadedPostView } from '$lib/fragments'; + import { SettingsTile } from '$lib/fragments';
33-41:⚠️ Potential issue | 🟠 MajorNo state cleanup or navigation after successful post creation.
After
createPostsucceeds,uploadedImagesis never cleared and there is nogoto(...)call. The user remains on this page with stale images. The$effecton lines 11-15 would navigate back only ifuploadedImageswere emptied. Compare withCreatePostModal.handleSubmitwhich resetstext,images,imageFiles, anderroron success.Suggested fix
const postSubmissionHandler = async () => { if (!uploadedImages.value) return; const images = uploadedImages.value.map((img) => img.url); try { await createPost(caption, images); + uploadedImages.value = []; + caption = ''; } catch (error) { console.error('Failed to create post:', error); } };
🤖 Fix all issues with AI agents
In `@platforms/pictique/src/lib/fragments/CreatePostModal/CreatePostModal.svelte`:
- Around line 46-48: images and imageFiles are updated separately and can get
out of sync; change to a single array (e.g., uploadItems) of objects like {
file: File, dataUrl: string } and only append to it inside reader.onload after
both file and dataUrl are ready (remove the early imageFiles push at the top of
the handler). Update removeImage(index) and any other places that splice or
reference imageFiles/images to operate on uploadItems (and handle read errors by
not appending and setting error). Also apply the same single-array approach for
the other upload code paths noted around the 61-64 area so all read+file data
are added atomically in the reader.onload callback.
- Around line 78-79: The catch uses a parameter named error which shadows the
component-level let error = $state('') in CreatePostModal.svelte, so failures
are only logged and not shown to the user; rename the catch parameter (e.g., to
err or creationError), assign the component error state (the let error variable)
to a user-friendly message derived from the caught err (err.message or
String(err)), and keep the console logging for debugging; update the catch block
around the post creation logic to set the component-level error so the existing
UI error display works.
- Line 116: The {`#each` images as image, index (index)} loop in
CreatePostModal.svelte uses the array index as the key causing DOM reuse issues;
change the key to a stable identifier (e.g., use the image data URL or an
assigned id: {`#each` images as image (image.url)} or (image.id)). If your image
objects are currently plain strings, wrap them into objects with a unique id at
upload time (assign in your upload/insert handler such as the function that
pushes into images) so the each block can use image.id as the key.
In
`@platforms/pictique/src/lib/fragments/UploadedPostView/UploadedPostView.svelte`:
- Line 27: The {`#each`} in UploadedPostView.svelte currently uses the array index
as the key ("{`#each` images as image, i (i)}"), which causes incorrect DOM reuse
when items are removed; change the key to a stable identifier such as image.url
by replacing the index-key usage with a stable key expression (e.g., use
image.url) in the {`#each` images as image, i (...) } block and, if image.url may
be missing, fall back to a stable unique property (like image.id) or generate a
stable key before rendering.
In `@platforms/pictique/src/routes/`(protected)/messages/[id]/+page.svelte:
- Around line 195-197: The placeholder "No messages yet" flashes because
messages starts as [] until SSE delivers history; add a boolean flag (e.g., let
connected = false or let historyLoaded = false) and set it when the SSE connects
or after the first onmessage in watchEventStream (or in the SSE onopen handler),
then change the conditional to render the placeholder only when historyLoaded is
true && messages.length === 0; also remove the redundant !messages check since
messages is always an array (reference the messages variable and the
watchEventStream / SSE onopen/onmessage handlers to locate where to set the
flag).
In `@platforms/pictique/src/routes/`(protected)/post/+page.svelte:
- Around line 17-31: The handleImageUpload function lacks file-size validation;
replicate the same rules from CreatePostModal (max 1MB per file and 5MB total)
by (1) checking file.size before reading and rejecting files >1MB, (2) computing
currentTotal by summing a new size property on uploadedImages.value items plus
the incoming file.size to ensure total <=5MB, and (3) storing size on each
uploaded image object (e.g., { url, alt, size }) so subsequent uploads can
validate against the accumulated total; update uploadedImages usage and the
FileReader flow in handleImageUpload to enforce and abort on violations.
- Line 46: The {`#each` uploadedImages.value ?? [] as image, i (i)} loop uses the
index as the key which can cause DOM reuse bugs when images are added/removed;
update the each block to use a stable unique identifier like image.url (i.e.,
change the key from i to image.url) so the loop is keyed by uploadedImages.value
and image.url to ensure correct DOM updates for the uploadedImages.value array
and each image item.
In
`@platforms/pictique/src/routes/`(protected)/settings/account/username/+page.svelte:
- Around line 46-48: The base64 avatar upload via apiClient.patch with
profileImageDataUrl can exceed the server proxy ~1MB limit because base64
inflates size; update the upload flow to avoid base64: either lower
MAX_FILE_SIZE constant (reference MAX_FILE_SIZE used in the upload/validation
logic) to ~750KB so encoded payload stays under the proxy limit, or change the
request in the component that calls apiClient.patch('/api/users/', ...) to send
the avatar as multipart/form-data (send the raw File/Blob instead of
profileImageDataUrl) and adjust server/client headers accordingly; locate the
validation and submit logic around profileImageDataUrl and apiClient.patch to
implement the chosen fix.
🧹 Nitpick comments (10)
platforms/pictique/src/lib/fragments/SideBar/SideBar.svelte (1)
167-167: Minor inconsistency: only the Profile label usesmt-1while the other four tabs still usemt-[4px].Both are equivalent (4px), but the sidebar now mixes arbitrary and standard utility for the same spacing across sibling elements (lines 72, 95, 118, 141 vs. 167). Consider updating the other tabs to
mt-1as well for consistency.♻️ Suggested diff to unify margin class across all tabs
<h3 - class={`${activeTab === 'home' ? 'text-brand-burnt-orange' : 'text-black-800'} mt-[4px]`} + class={`${activeTab === 'home' ? 'text-brand-burnt-orange' : 'text-black-800'} mt-1`} > Feed </h3><h3 - class={`${activeTab === 'discover' ? 'text-brand-burnt-orange' : 'text-black-800'} mt-[4px]`} + class={`${activeTab === 'discover' ? 'text-brand-burnt-orange' : 'text-black-800'} mt-1`} > Search </h3><h3 - class={`${activeTab === 'messages' ? 'text-brand-burnt-orange' : 'text-black-800'} mt-[4px]`} + class={`${activeTab === 'messages' ? 'text-brand-burnt-orange' : 'text-black-800'} mt-1`} > Messages </h3><h3 - class={`${activeTab === 'settings' ? 'text-brand-burnt-orange' : 'text-black-800'} mt-[4px]`} + class={`${activeTab === 'settings' ? 'text-brand-burnt-orange' : 'text-black-800'} mt-1`} > Settings </h3>platforms/pictique/src/lib/stores/posts.ts (2)
78-78:images.map((img) => img)is a no-op identity map.This creates a shallow copy of the array without transforming any elements. If the intent is to clone the array, use
[...images]; otherwise, just passimagesdirectly.♻️ Suggested simplification
const payload = { text, - images: images.map((img) => img) + images };
81-83: Debugconsole.logwill ship to production.This logging is helpful during development, but leaving it in a PR targeting
mainwill add noise to end-user browser consoles on every post creation. Consider removing it before merge, or gating it behind an environment/debug flag.♻️ Option A: Remove the debug log
- // Log payload size for debugging - const payloadSize = new Blob([JSON.stringify(payload)]).size; - console.log(`Payload size: ${(payloadSize / 1024).toFixed(2)} KB (${images.length} images)`); - const response = await apiClient.post('/api/posts', payload);♻️ Option B: Gate behind a dev-only check
- // Log payload size for debugging - const payloadSize = new Blob([JSON.stringify(payload)]).size; - console.log(`Payload size: ${(payloadSize / 1024).toFixed(2)} KB (${images.length} images)`); + if (import.meta.env.DEV) { + const payloadSize = new Blob([JSON.stringify(payload)]).size; + console.log(`Payload size: ${(payloadSize / 1024).toFixed(2)} KB (${images.length} images)`); + }platforms/pictique/src/lib/fragments/MessageInput/MessageInput.svelte (2)
60-70: Stale a11y-ignore comments and tab character in class string.Now that the raw
<div>has been replaced with a<Button>component, thea11y_click_events_have_key_eventsanda11y_no_static_element_interactionssuppression comments on lines 60–61 are likely unnecessary — a<button>element inherently satisfies both rules.Also, line 63 contains a stray tab character (
\t) between"bg-greyandflexin the class string.Suggested cleanup
- <!-- svelte-ignore a11y_click_events_have_key_events --> - <!-- svelte-ignore a11y_no_static_element_interactions --> <Button - class="bg-grey flex aspect-square h-13 w-13 items-center justify-center rounded-full px-0 transition-opacity {isDisabled + class="bg-grey flex aspect-square h-13 w-13 items-center justify-center rounded-full px-0 transition-opacity {isDisabled ? 'cursor-not-allowed opacity-50' : 'cursor-pointer hover:opacity-80'}" callback={handleSubmit}
3-3: Inconsistent import style.
Buttonis imported directly from its file path whileAvatarandInputon line 2 use the barrel export ($lib/ui). Consider importingButtonfrom$lib/uias well for consistency, unless there's a specific reason (e.g., circular dependency).- import Button from '$lib/ui/Button/Button.svelte'; + import { Avatar, Button, Input } from '$lib/ui';(and drop the separate
Avatar/Inputimport on line 2.)platforms/pictique/src/routes/(protected)/settings/account/username/+page.svelte (1)
54-55: Axios errors carry the message in a nested property.
err.messageon anAxiosErroryields a generic string like"Request failed with status code 413". If the server returns a meaningful error body, extract it witherr.response?.data?.message(or equivalent) before falling back toerr.message.Suggested improvement
- error = err instanceof Error ? err.message : 'Failed to save changes'; + const axiosErr = err as any; + error = axiosErr?.response?.data?.message + ?? (err instanceof Error ? err.message : 'Failed to save changes');platforms/pictique/src/lib/fragments/CreatePostModal/CreatePostModal.svelte (1)
161-178: Progress bar hidden for the first image.The
{#ifimages.length > 1}guard means users won't see any size feedback until the second upload. Consider showing it whenimages.length > 0so users know how much budget a single large image consumed.platforms/pictique/src/lib/fragments/UploadedPostView/UploadedPostView.svelte (2)
7-20:widthandheightprops are now unused.These props are still declared in the interface (lines 9-10) and destructured with defaults (lines 16-17), but the template no longer references them — the grid uses fixed Tailwind classes. Remove them to avoid confusion, and update the component docs (lines 50-51) accordingly.
Suggested cleanup
interface IUploadedPostViewProps extends HTMLAttributes<HTMLElement> { images: Image[]; - width?: string; - height?: string; callback: (i: number) => void; } let { images, - width = 'w-full', - height = 'h-full', callback, ...restProps }: IUploadedPostViewProps = $props();
25-25: Simplifycncall.Wrapping a
.join(' ')result in an array is redundant —cnaccepts multiple string arguments directly.Simplification
- class={cn(['grid grid-cols-2 gap-4 md:grid-cols-3 lg:grid-cols-4', restProps.class].join(' '))} + class={cn('grid grid-cols-2 gap-4 md:grid-cols-3 lg:grid-cols-4', restProps.class)}platforms/pictique/src/routes/(protected)/post/+page.svelte (1)
51-56:revokeImageUrlsis a no-op on data URLs.Images here are read via
FileReader.readAsDataURL, producingdata:URLs.URL.revokeObjectURLonly applies to blob URLs created byURL.createObjectURL. This call does nothing — it won't break, but it's misleading. Consider either switching toURL.createObjectURLfor the preview (more memory-efficient) or removing the revoke call.
platforms/pictique/src/lib/fragments/CreatePostModal/CreatePostModal.svelte
Show resolved
Hide resolved
platforms/pictique/src/lib/fragments/CreatePostModal/CreatePostModal.svelte
Outdated
Show resolved
Hide resolved
platforms/pictique/src/lib/fragments/CreatePostModal/CreatePostModal.svelte
Outdated
Show resolved
Hide resolved
platforms/pictique/src/lib/fragments/UploadedPostView/UploadedPostView.svelte
Outdated
Show resolved
Hide resolved
platforms/pictique/src/routes/(protected)/messages/[id]/+page.svelte
Outdated
Show resolved
Hide resolved
platforms/pictique/src/routes/(protected)/settings/account/username/+page.svelte
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
platforms/pictique/src/routes/(protected)/post/+page.svelte (1)
60-68:⚠️ Potential issue | 🔴 Critical
catch (error)shadows the component-levelerrorstate — submission failures are silently swallowed.The catch parameter
erroron line 65 shadowslet error = $state('')on line 14. The caught exception is only logged; the UI error banner (lines 72-76) will never display post-creation failures.Proposed fix
- } catch (error) { - console.error('Failed to create post:', error); + } catch (err) { + console.error('Failed to create post:', err); + error = err instanceof Error ? err.message : 'Failed to create post'; }platforms/pictique/src/routes/(protected)/settings/account/username/+page.svelte (1)
132-132:⚠️ Potential issue | 🟡 MinorTypo: double space in placeholder text.
"Edit your public name"has two spaces between "Edit" and "your".Proposed fix
- <Input type="text" placeholder="Edit your public name" bind:value={name} /> + <Input type="text" placeholder="Edit your public name" bind:value={name} />
🤖 Fix all issues with AI agents
In `@platforms/pictique/src/lib/fragments/CreatePostModal/CreatePostModal.svelte`:
- Line 44: In CreatePostModal.svelte remove the debug statement that logs the
full base64 payload—delete the console.log(result) call (or replace it with a
safe metadata-only log such as filename/size/type) inside the image/attachment
handling code so you no longer dump full data URLs to the console; look for the
console.log(result) occurrence in the component's image processing function and
remove or replace it with a minimal log of non-sensitive metadata.
In
`@platforms/pictique/src/routes/`(protected)/settings/account/username/+page.svelte:
- Line 89: The UI helper text in +page.svelte incorrectly states "Maximum file
size: 1MB" while the actual limit is defined by MAX_FILE_SIZE (0.5 * 1024 *
1024). Update the displayed text to match the constant (e.g., "Maximum file
size: 500KB" or derive the value from MAX_FILE_SIZE) so the text and the
MAX_FILE_SIZE value are consistent; locate the string in +page.svelte and
replace it or format it from MAX_FILE_SIZE to ensure parity with the upload
limit.
🧹 Nitpick comments (6)
platforms/pictique/src/routes/(protected)/post/+page.svelte (3)
10-11:MAX_TOTAL_SIZEandMAX_FILE_SIZEare duplicated across files.These same constants are defined identically in
CreatePostModal.svelte. Consider exporting them fromfileValidation.tsalongside the validation functions to keep a single source of truth.
84-89:revokeImageUrlsis a no-op on data URLs.Images in this flow are created via
FileReader.readAsDataURL, producingdata:URIs — not object URLs fromURL.createObjectURL(). CallingrevokeObjectURLon a data URL does nothing. Not harmful, but misleading.
78-133: Add Photo frame is always visible, even when total size limit is reached.Unlike
CreatePostModalwhich conditionally renders the frame with{#ifremainingSize > 0}, this page always shows it. Users will see the frame, attempt an upload, and only then get an error. Consider hiding it when the budget is exhausted, consistent withCreatePostModal.platforms/pictique/src/routes/(protected)/settings/account/username/+page.svelte (1)
30-38: Missingreader.onerrorhandler.If
FileReader.readAsDataURLfails, the error is silently swallowed. Other upload flows in this PR (e.g.,CreatePostModal,post/+page.svelte) includereader.onerrorto surface the failure. Add the same here for consistency.Proposed fix
reader.onload = (e) => { if (e.target?.result) { profileImageDataUrl = e.target.result as string; } }; + reader.onerror = () => { + error = 'Error reading image file'; + }; reader.readAsDataURL(file);platforms/pictique/src/lib/fragments/UploadedPostView/UploadedPostView.svelte (1)
7-20:widthandheightprops are declared but no longer used.The interface declares
widthandheight(with defaults), and they're destructured from$props(), but the template now uses a grid layout that doesn't reference them. This is dead code that may mislead consumers of the component.Proposed cleanup
interface IUploadedPostViewProps extends HTMLAttributes<HTMLElement> { images: Image[]; - width?: string; - height?: string; callback: (i: number) => void; } let { images, - width = 'w-full', - height = 'h-full', callback, ...restProps }: IUploadedPostViewProps = $props();Also update the documentation block (lines 50–51) to remove references to
widthandheight.platforms/pictique/src/lib/fragments/CreatePostModal/CreatePostModal.svelte (1)
63-80: NoisSubmittingguard at the top ofhandleSubmit.The
disabledprop on the Post button (line 184) doesn't account forisSubmitting, so rapid clicks before state propagates could trigger concurrent submissions. Add an early return, consistent withsaveProfileDatain the settings page.Proposed fix
const handleSubmit = async () => { + if (isSubmitting) return; if (!text.trim() && uploadItems.length === 0) return;And also disable the button during submission:
- disabled={!text.trim() && uploadItems.length === 0} + disabled={isSubmitting || (!text.trim() && uploadItems.length === 0)}
platforms/pictique/src/lib/fragments/CreatePostModal/CreatePostModal.svelte
Outdated
Show resolved
Hide resolved
platforms/pictique/src/routes/(protected)/settings/account/username/+page.svelte
Outdated
Show resolved
Hide resolved
| const MAX_TOTAL_SIZE = 5 * 1024 * 1024; // 5MB total | ||
| const MAX_FILE_SIZE = 1 * 1024 * 1024; // 1MB per individual image |
There was a problem hiding this comment.
ah well, we don't really want to enforce these that hard
2 participants
Description of change
UI/UX Improvements
Upload Validation
Bug Fixes
isSubmittingstate with proper async handlingTechnical
Issue Number
Closes #732
Type of change
How the change has been tested
Change checklist
Summary by CodeRabbit
New Features
Style
Accessibility
Bug Fixes