-
Notifications
You must be signed in to change notification settings - Fork 4
feat/upload-post #190
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
feat/upload-post #190
Conversation
WalkthroughThis update introduces a photo upload modal to the protected layout, enabling users to select images, preview them, and add captions within a modal interface. Supporting changes include enhancements to the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant SideBar
participant Layout
participant Modal
participant InputFile
participant Textarea
User->>SideBar: Click "Post" action
SideBar->>Layout: handlePost callback
Layout->>Modal: Present modal (paneModal.present())
Modal->>InputFile: Render file input if no files selected
User->>InputFile: Select image files
InputFile->>Layout: Update files state
Layout->>Layout: Generate image previews
Modal->>Layout: Display image previews
User->>Modal: Click "Next"
Modal->>Textarea: Show caption input
User->>Textarea: Enter caption
User->>Modal: Click "Cancel" or dismiss modal
Modal->>Layout: handleDismiss, clear files/caption
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 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 (1)
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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
🧹 Nitpick comments (7)
platforms/metagram/src/lib/ui/Textarea/Textarea.svelte (3)
18-20
: Consider breaking down the long CSS class string for better maintainability.The CSS class string is difficult to read and maintain. Consider using a multi-line template literal or extracting related classes into semantic groups.
- const cbase = $derived( - 'w-full bg-grey resize-none py-3.5 px-6 text-[15px] text-black-800 font-geist font-normal placeholder:text-black-600 rounded-4xl outline-0 border border-transparent invalid:border-red invalid:text-red focus:invalid:text-black-800 focus:invalid:border-transparent' - ); + const cbase = $derived(` + w-full bg-grey resize-none + py-3.5 px-6 + text-[15px] text-black-800 font-geist font-normal + placeholder:text-black-600 + rounded-4xl outline-0 + border border-transparent + invalid:border-red invalid:text-red + focus:invalid:text-black-800 focus:invalid:border-transparent + `);
30-30
: Simplify the class name concatenation.The
cn
utility function likely already handles arrays, so the manual join is redundant.- class={cn([cbase, restProps.class].join(' '))} + class={cn(cbase, restProps.class)}
31-31
: Remove unnecessary tabindex attribute.Textarea elements are naturally focusable and don't need an explicit
tabindex="0"
.- tabindex="0"
platforms/metagram/src/lib/fragments/InputFile/InputFile.svelte (2)
29-30
: Useconst
instead oflet
for immutable variable.The
cBase
variable is never reassigned, so it should be declared withconst
for better code clarity.- let cBase = + const cBase =
43-43
: Simplify the class name concatenation.The
cn
utility function likely already handles arrays, so the manual join is redundant.-<label {...restProps} for={uniqueId} class={cn([cBase, restProps.class].join(' '))}> +<label {...restProps} for={uniqueId} class={cn(cBase, restProps.class)}>platforms/metagram/src/lib/fragments/Modal/Modal.svelte (1)
75-75
: Useoverflow: auto
instead ofoverflow: scroll
.Using
auto
will only show scrollbars when content overflows, providing a better user experience.- overflow: scroll !important; + overflow: auto !important;platforms/metagram/src/routes/(protected)/+layout.svelte (1)
209-212
: Avoid hardcoding dimensions in multiple places.The image dimensions are hardcoded in both classes and inline styles. Consider using CSS variables or extracting to constants.
- <div class="h-[100px] w-[80px] overflow-hidden rounded-lg border"> + <div class="aspect-[4/5] h-[100px] overflow-hidden rounded-lg border"> <!-- svelte-ignore a11y_img_redundant_alt --> - <img {src} alt="Selected image" class="h-[100px] w-[80px] object-cover" /> + <img {src} alt="Selected image" class="h-full w-full object-cover" /> </div>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
platforms/metagram/src/lib/fragments/InputFile/InputFile.svelte
(1 hunks)platforms/metagram/src/lib/fragments/Modal/Modal.svelte
(3 hunks)platforms/metagram/src/lib/fragments/UserRequest/UserRequest.svelte
(1 hunks)platforms/metagram/src/lib/ui/Textarea/Textarea.stories.ts
(1 hunks)platforms/metagram/src/lib/ui/Textarea/Textarea.svelte
(1 hunks)platforms/metagram/src/lib/ui/index.ts
(1 hunks)platforms/metagram/src/routes/(protected)/+layout.svelte
(4 hunks)platforms/metagram/src/routes/(protected)/discover/+page.svelte
(1 hunks)platforms/metagram/src/routes/+layout.svelte
(1 hunks)
🔇 Additional comments (4)
platforms/metagram/src/lib/ui/index.ts (1)
8-8
: Export new Textarea component
TheTextarea
component is correctly added to the UI index with a consistent naming convention. Ensure that downstream imports are updated to use this consolidated export.platforms/metagram/src/routes/(protected)/discover/+page.svelte (1)
10-23
: Formatting cleanup in conditional rendering
Indentation and class‐attribute reordering within the{#if searchValue}
block are purely presentational. No logic or behavior changes.platforms/metagram/src/lib/fragments/UserRequest/UserRequest.svelte (1)
18-31
: Approve formatting adjustments
Reordered class names (items-center justify-between
) and updated indentation for improved readability. No functional changes detected.platforms/metagram/src/routes/+layout.svelte (1)
37-42
: Syntax consistency refinements
Added semicolons insetTimeout
/onMount
blocks, normalized self‐closing<img />
syntax, and adjusted element class ordering. No impact on runtime behavior.Also applies to: 46-51
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
♻️ Duplicate comments (3)
platforms/metagram/src/routes/(protected)/+layout.svelte (3)
196-196
: Fix the accept attribute value.
88-102
: Add error handling for file reading operations.
192-192
: Reset all modal state when dismissing.
🧹 Nitpick comments (3)
platforms/metagram/src/routes/(protected)/+layout.svelte (3)
219-224
: Improve radio button ID generation for stability.Using
option + i
for IDs could lead to issues if the options array changes. Consider using a more stable identifier.- {#each postVisibilityOptions as option,i} + {#each postVisibilityOptions as option} <div class="flex items-center justify-between w-[50%] mb-2"> - <Label for={option + i}>{option}</Label> - <InputRadio name="post-visibility" id={option + i} value={option} bind:selected={postVisibility}/> + <Label for={`visibility-${option.replace(/\s+/g, '-')}`}>{option}</Label> + <InputRadio name="post-visibility" id={`visibility-${option.replace(/\s+/g, '-')}`} value={option} bind:selected={postVisibility}/> </div> {/each}
208-209
: Add validation and accessibility to caption input.Consider adding character limits and proper labeling for the caption textarea.
- <Label>Add a Caption</Label> - <Textarea class="mb-4" bind:value={caption} placeholder="enter caption" /> + <Label for="caption-input">Add a Caption</Label> + <Textarea + id="caption-input" + class="mb-4" + bind:value={caption} + placeholder="Enter caption (max 2200 characters)" + maxlength={2200} + />
201-202
: Improve image alt text for accessibility.Consider providing more descriptive alt text that includes the image index or unique identifier.
- <img {src} alt="Selected image" class="h-full w-full object-cover" /> + <img {src} alt={`Selected image ${imagePreviews.indexOf(src) + 1} of ${imagePreviews.length}`} class="h-full w-full object-cover" />Apply similar changes to the caption view images at line 214.
Also applies to: 214-214
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
platforms/metagram/src/lib/fragments/Modal/Modal.svelte
(3 hunks)platforms/metagram/src/lib/ui/InputRadio/InputRadio.svelte
(2 hunks)platforms/metagram/src/lib/ui/index.ts
(1 hunks)platforms/metagram/src/routes/(protected)/+layout.svelte
(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- platforms/metagram/src/lib/ui/index.ts
- platforms/metagram/src/lib/fragments/Modal/Modal.svelte
🔇 Additional comments (1)
platforms/metagram/src/lib/ui/InputRadio/InputRadio.svelte (1)
9-9
: LGTM! Good accessibility enhancement.The addition of the optional
id
prop properly enables external labeling and improves accessibility of the radio input component. The implementation correctly follows Svelte patterns.Also applies to: 16-16, 28-28
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
🧹 Nitpick comments (5)
platforms/metagram/src/lib/fragments/AddPostModal/AddPostModal.svelte (5)
44-46
: Simplify the dismiss handler syntax.The comma operator usage is valid but unconventional. Consider using a more explicit approach for better readability.
handleDismiss={() => { - (files = undefined), (isAddCaption = false); + files = undefined; + isAddCaption = false; + caption = ''; }}
56-57
: Remove unnecessary accessibility ignore comments.The alt text "Selected image" is descriptive and appropriate. The
svelte-ignore a11y_img_redundant_alt
comments are unnecessary.- <!-- svelte-ignore a11y_img_redundant_alt --> <img {src} alt="Selected image" class="h-full w-full object-cover" />
- <!-- svelte-ignore a11y_img_redundant_alt --> <img {src} alt="Selected image" class="h-[100px] w-[80px] object-cover" />
Also applies to: 68-69
67-70
: Fix image aspect ratio preservation.The fixed dimensions on the caption step images may distort the aspect ratio. Consider using
object-cover
with flexible containers.- <div class="h-[100px] w-[80px] overflow-hidden rounded-lg border"> - <img {src} alt="Selected image" class="h-[100px] w-[80px] object-cover" /> + <div class="aspect-[4/5] w-[80px] overflow-hidden rounded-lg border"> + <img {src} alt="Selected image" class="h-full w-full object-cover" /> </div>
20-20
: Extract post visibility options to a constant.Consider extracting the hardcoded options to improve maintainability and enable potential internationalization.
+ const POST_VISIBILITY_OPTIONS = ['Only followers', 'Close friends', 'Anyone'] as const; - let postVisibilityOptions = ['Only followers', 'Close friends', 'Anyone']; + let postVisibilityOptions = POST_VISIBILITY_OPTIONS;
97-103
: Add validation and submission functionality.The "Next" button allows proceeding to the caption step without any validation, and there's no final submission functionality. Consider adding validation and a submit action.
The modal currently lacks:
- Validation to ensure files are selected before proceeding
- A final submit/post action after caption and visibility are set
- Loading states during submission
Would you like me to help implement these missing features?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
platforms/metagram/src/lib/fragments/AddPostModal/AddPostModal.svelte
(1 hunks)platforms/metagram/src/lib/fragments/index.ts
(1 hunks)platforms/metagram/src/routes/(protected)/+layout.svelte
(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- platforms/metagram/src/lib/fragments/index.ts
- platforms/metagram/src/routes/(protected)/+layout.svelte
Description of change
Built a upload a post modal and text area
Issue Number
N/A
Type of change
How the change has been tested
CI and Manual
Change checklist
Summary by CodeRabbit
New Features
Enhancements
Documentation
Style