Feat/preview post #418
Feat/preview post #418
Conversation
|
@yurimutti is attempting to deploy a commit to the Listinai Team on Vercel. A member of the Team first needs to authorize it. |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
…nto feat/preview-post
|
Hi @yurimutti, are you still working on it? |
Hello @nevo-david I waiting for a review. |
I resolved the conflicts. |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
|
Important Review skippedMore than 25% of the files skipped due to max files limit. The review is being skipped to prevent a low-quality review. 580 files out of 692 files are above the max files limit of 100. You can disable this status message by setting the WalkthroughThe pull request introduces several updates across multiple components in a Next.js application. A new Changes
Possibly related PRs
Poem
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (6)
apps/frontend/src/app/(site)/preview/[id]/page.tsx (1)
7-10: Consider adding dynamic metadata based on post content.While the current metadata is functional, consider enhancing it by including post-specific information (title, description) once the data is fetched.
Example implementation:
export async function generateMetadata({ params }: { params: { id: string } }): Promise<Metadata> { try { const post = await fetch(`/api/posts/${params.id}`).then(res => res.json()); return { title: `${isGeneralServerSide() ? 'Postiz' : 'Gitroom'} - ${post.title}`, description: post.description || 'Make a preview link for your posts.', }; } catch (error) { return { title: `${isGeneralServerSide() ? 'Postiz' : 'Gitroom'} Preview`, description: 'Make a preview link for your posts.', }; } }apps/frontend/src/components/preview/preview.tsx (1)
30-35: Enhance error handling and user feedback.The current error messages could be more descriptive and user-friendly.
- <main className="flex mx-auto"> - <h1>{!data?.posts ? 'No post founded.' : 'Oops! Something went wrong.'} </h1> + <main className="flex flex-col items-center justify-center min-h-[200px] mx-auto"> + <h1 className="text-xl font-semibold mb-2"> + {!data?.posts ? 'Post not found' : 'Unable to load preview'} + </h1> + <p className="text-gray-600"> + {!data?.posts + ? 'The post you're looking for might have been deleted or is not accessible.' + : 'Please try again later or contact support if the issue persists.'} + </p> </main>apps/frontend/src/components/launches/general.preview.component.tsx (3)
Line range hint
13-21: Extract text processing logic to a custom hook.The text slicing and formatting logic would be more maintainable as a separate hook.
// hooks/useTextProcessing.ts export const useTextProcessing = (text: string, maxChars: number, identifier: string) => { return useMemo(() => { const {start, end} = textSlicer(identifier, maxChars, text); return { processedText: text.slice(start, end), croppedText: text.slice(end), isCropped: end < text.length }; }, [text, maxChars, identifier]); };
Line range hint
82-82: Security Risk: Unsafe HTML rendering.Using
dangerouslySetInnerHTMLwith user-generated content poses XSS risks. Consider using a sanitization library.+import DOMPurify from 'dompurify'; -<pre className={clsx('text-wrap', chakra.className)} dangerouslySetInnerHTML={{__html: value.text}} /> +<pre className={clsx('text-wrap', chakra.className)} + dangerouslySetInnerHTML={{__html: DOMPurify.sanitize(value.text)}} />
Line range hint
83-102: Add error handling for images.The image rendering lacks error handling and loading states.
<VideoOrImage autoplay={true} src={mediaDir.set(image.path)} + onError={(e) => { + e.currentTarget.src = '/fallback-image.png'; + }} + loading="lazy" />apps/frontend/src/components/launches/providers/high.order.provider.tsx (1)
411-434: Consider enhancing the Share button accessibilityWhile the implementation is good, consider adding ARIA attributes for better accessibility.
Add these attributes to improve accessibility:
<Button onClick={handleShare} + aria-label="Share post" className="!h-[24px] rounded-[3px] flex gap-[4px] w-[102px] text-[12px] font-[500]" >
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (8)
apps/frontend/src/app/(site)/preview/[id]/page.tsx(1 hunks)apps/frontend/src/components/launches/add.edit.model.tsx(3 hunks)apps/frontend/src/components/launches/calendar.tsx(3 hunks)apps/frontend/src/components/launches/general.preview.component.tsx(2 hunks)apps/frontend/src/components/launches/launches.component.tsx(1 hunks)apps/frontend/src/components/launches/providers.options.tsx(1 hunks)apps/frontend/src/components/launches/providers/high.order.provider.tsx(8 hunks)apps/frontend/src/components/preview/preview.tsx(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- apps/frontend/src/components/launches/providers.options.tsx
- apps/frontend/src/components/launches/calendar.tsx
🔇 Additional comments (6)
apps/frontend/src/app/(site)/preview/[id]/page.tsx (2)
5-5: LGTM! Dynamic rendering is appropriate for preview pages.
The force-dynamic setting ensures fresh data on each request, which is suitable for preview functionality.
12-16: LGTM! Clean and focused component implementation.
The component follows Next.js patterns and properly passes the ID parameter to the Preview component.
apps/frontend/src/components/preview/preview.tsx (1)
18-21:
Add missing dependency to useCallback.
The fetch function is used within the callback but not included in the dependency array.
}, [id]);
+ }, [id, fetch]);apps/frontend/src/components/launches/launches.component.tsx (1)
153-153: LGTM!
Minor whitespace adjustment with no functional impact.
apps/frontend/src/components/launches/providers/high.order.provider.tsx (1)
222-234: LGTM: Share functionality implementation
The handleShare function is well-implemented with:
- Proper URL construction using window.location.origin
- Error handling with user feedback
- Clipboard API usage
apps/frontend/src/components/launches/add.edit.model.tsx (1)
Line range hint 596-630: LGTM: Improved button section UI
The changes enhance the button section with:
- Better conditional text rendering
- Improved text alignment
- Consistent styling
A1exKH
left a comment
A1exKH
left a comment
There was a problem hiding this comment.
@yurimutti this PR LGTM.
Could you resolve conflicts for this PR, please?
Done. Could you review again? |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (1)
apps/frontend/src/components/launches/providers/high.order.provider.tsx (1)
448-471: Improve Share button accessibility and maintainabilityThe Share button implementation could be enhanced in several ways:
- Add accessibility attributes
- Extract SVG to a separate component
- Make the button width responsive
- Add loading state during clipboard operation
Consider these improvements:
<Button onClick={handleShare} - className="!h-[24px] rounded-[3px] flex gap-[4px] w-[102px] text-[12px] font-[500]" + className="!h-[24px] rounded-[3px] flex gap-[4px] min-w-[102px] text-[12px] font-[500]" + aria-label="Share post" + disabled={!existingData?.posts?.length} > - <div> - <svg - xmlns="http://www.w3.org/2000/svg" - version="1.1" - x="0px" - y="0px" - viewBox="0 0 800 1000" - className="max-w-[20px] max-h-[20px] fill-current h-[1.25em]" - > - <path - fill="white" - d="M443.25,529.42c2.9,1.32,6.32,0.83,8.73-1.27l211.28-183.37c1.79-1.55,2.81-3.79,2.81-6.16c0-2.37-1.02-4.61-2.81-6.16 L451.98,149.11c-2.41-2.1-5.83-2.59-8.73-1.27c-2.91,1.33-4.77,4.23-4.77,7.42v109.52c-168.67,5.27-304.54,173.69-304.54,379.94 c0,3.81,2.63,7.11,6.35,7.95c0.61,0.14,1.21,0.21,1.81,0.21c3.08,0,5.97-1.75,7.35-4.63c81.79-170.48,158.11-233.12,289.03-235.58 V522C438.47,525.19,440.34,528.09,443.25,529.42z M151.83,607.06c15.41-182.94,141.74-326.08,294.8-326.08 c4.51,0,8.16-3.65,8.16-8.16v-99.67l190.67,165.48L454.79,504.11v-99.67c0-4.51-3.65-8.16-8.16-8.16 C314.35,396.29,231.89,454.14,151.83,607.06z" - /> - </svg> - </div> + <ShareIcon className="max-w-[20px] max-h-[20px] fill-current h-[1.25em]" /> <div className="text-white">Share</div> </Button>Create a new ShareIcon component:
// components/icons/ShareIcon.tsx export const ShareIcon: React.FC<{ className?: string }> = ({ className }) => ( <svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 800 1000" className={className} aria-hidden="true" > <path fill="currentColor" d="M443.25,529.42c2.9,1.32,6.32,0.83,8.73-1.27l211.28-183.37c1.79-1.55,2.81-3.79,2.81-6.16c0-2.37-1.02-4.61-2.81-6.16 L451.98,149.11c-2.41-2.1-5.83-2.59-8.73-1.27c-2.91,1.33-4.77,4.23-4.77,7.42v109.52c-168.67,5.27-304.54,173.69-304.54,379.94 c0,3.81,2.63,7.11,6.35,7.95c0.61,0.14,1.21,0.21,1.81,0.21c3.08,0,5.97-1.75,7.35-4.63c81.79-170.48,158.11-233.12,289.03-235.58 V522C438.47,525.19,440.34,528.09,443.25,529.42z M151.83,607.06c15.41-182.94,141.74-326.08,294.8-326.08 c4.51,0,8.16-3.65,8.16-8.16v-99.67l190.67,165.48L454.79,504.11v-99.67c0-4.51-3.65-8.16-8.16-8.16 C314.35,396.29,231.89,454.14,151.83,607.06z" /> </svg> );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/frontend/src/components/launches/add.edit.model.tsx(3 hunks)apps/frontend/src/components/launches/calendar.tsx(3 hunks)apps/frontend/src/components/launches/general.preview.component.tsx(2 hunks)apps/frontend/src/components/launches/providers/high.order.provider.tsx(8 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/frontend/src/components/launches/calendar.tsx
- apps/frontend/src/components/launches/general.preview.component.tsx
- apps/frontend/src/components/launches/add.edit.model.tsx
🔇 Additional comments (2)
apps/frontend/src/components/launches/providers/high.order.provider.tsx (2)
38-39: Clean up unused imports and variables
The static analysis tool has identified several unused declarations:
- 'Component' is defined but never used
- 'date' is defined but never used
- 'modal' is assigned but never used
Line range hint 229-471: Verify integration with the preview feature
Let's ensure that the share functionality properly integrates with the preview feature mentioned in the PR objectives.
✅ Verification successful
Based on the search results, I can now provide a final response:
Share functionality is properly integrated with preview feature
The implementation is correctly integrated with the preview feature:
- The Preview component exists and is properly implemented in
apps/frontend/src/components/preview/preview.tsx - The share functionality in
handleSharecorrectly constructs the preview URL using the post ID - The clipboard API is properly used with appropriate error handling and user feedback via toast messages
- The preview URL format (
/preview/${postId}) aligns with the existing preview component's route structure
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the preview route and component integration
# Check if the preview route exists
echo "Checking preview route..."
rg -l "preview/\[.*\]" --type tsx
# Check if the Preview component is properly imported and used
echo "Checking Preview component usage..."
ast-grep --pattern 'import { Preview } from "@gitroom/frontend/components/preview/preview"'
# Check for any existing share functionality to avoid duplication
echo "Checking for existing share functionality..."
rg -i "share.*link|copy.*link|clipboard" --type tsx
Length of output: 605
Script:
#!/bin/bash
# Let's try with more generic file extensions and patterns
# Check preview route and component implementation
echo "Checking preview route and component..."
rg -t js -t jsx -t ts -t tsx "preview/\[.*\]|route.*preview"
# Check the Preview component implementation
echo "Checking Preview component implementation..."
rg -t js -t jsx -t ts -t tsx -A 5 "export.*Preview"
# Check clipboard and share functionality
echo "Checking share functionality..."
rg -t js -t jsx -t ts -t tsx "navigator\.clipboard\.writeText|handleShare"
# Check if there are any other preview URLs being constructed
echo "Checking preview URL construction..."
rg -t js -t jsx -t ts -t tsx "preview/\$|/preview/.*id"
Length of output: 706
Script:
#!/bin/bash
# Let's try without file type restrictions since ripgrep seems to have issues with tsx/jsx
# Check preview route and component
echo "Checking preview route and component..."
rg "preview/\[.*\]|route.*preview"
# Check the Preview component
echo "Checking Preview component..."
rg -A 5 "export.*Preview"
# Check clipboard and share functionality
echo "Checking share functionality..."
rg "navigator\.clipboard\.writeText|handleShare"
# Check preview URL construction
echo "Checking preview URL construction..."
rg "/preview/.*id|preview/\$"
# Check if the preview page exists
echo "Checking preview page existence..."
fd "page.tsx" --full-path "preview"
Length of output: 3741
A1exKH
left a comment
A1exKH
left a comment
There was a problem hiding this comment.
@yurimutti please, could you resolve one ESLint warning?
After this PR can be approved.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/frontend/src/components/launches/providers/high.order.provider.tsx (1)
242-265: Consider enhancing the share functionalityThe implementation has good practices like safe URL construction and proper error handling. However, consider these improvements:
- Allow sharing specific posts instead of always sharing the first one
- Add more specific error messages for clipboard permission issues
const handleShare = async () => { if (!existingData.posts.length) { return toast.show('No posts available to share', 'warning'); } - const postId = existingData.posts[0].id; + // Allow sharing specific posts + const postId = existingData.posts[index].id; const previewPath = new URL( `/preview/${postId}`, window.location.origin ).toString(); try { if (!navigator.clipboard) { - throw new Error('Clipboard API not available'); + throw new Error('Clipboard access denied. Please check browser permissions.'); } await navigator.clipboard.writeText(previewPath); return toast.show('Link copied to clipboard.', 'success'); } catch (err) { if (err instanceof Error) - toast.show(`Failed to copy the link. ${err.message}`, 'warning'); + toast.show( + `Failed to copy the link. ${ + err.message.includes('denied') + ? 'Please allow clipboard access in your browser settings.' + : err.message + }`, + 'warning' + ); } };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/frontend/src/components/launches/add.edit.model.tsx(3 hunks)apps/frontend/src/components/launches/calendar.tsx(3 hunks)apps/frontend/src/components/launches/general.preview.component.tsx(2 hunks)apps/frontend/src/components/launches/providers.options.tsx(1 hunks)apps/frontend/src/components/launches/providers/high.order.provider.tsx(8 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- apps/frontend/src/components/launches/providers.options.tsx
- apps/frontend/src/components/launches/calendar.tsx
- apps/frontend/src/components/launches/add.edit.model.tsx
- apps/frontend/src/components/launches/general.preview.component.tsx
|
@A1exKH Hello, do we need more approval to forward this feature? |
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
apps/frontend/src/components/launches/providers/high.order.provider.tsx (1)
549-570: 🛠️ Refactor suggestionEnhance Share button accessibility
The Share button implementation needs accessibility improvements as previously suggested.
<Button onClick={handleShare} className="!h-[24px] rounded-[3px] flex gap-[4px] w-[102px] text-[12px] font-[500]" + aria-label="Share post" + role="button" > <div> <svg xmlns="http://www.w3.org/2000/svg" version="1.1" - x="0px" - y="0px" viewBox="0 0 800 1000" className="max-w-[20px] max-h-[20px] fill-current h-[1.25em]" + aria-hidden="true" + role="img" > <path fill="white" d="M443.25,529.42c2.9,1.32,6.32,0.83,8.73-1.27l211.28-183.37c1.79-1.55,2.81-3.79,2.81-6.16c0-2.37-1.02-4.61-2.81-6.16 L451.98,149.11c-2.41-2.1-5.83-2.59-8.73-1.27c-2.91,1.33-4.77,4.23-4.77,7.42v109.52c-168.67,5.27-304.54,173.69-304.54,379.94 c0,3.81,2.63,7.11,6.35,7.95c0.61,0.14,1.21,0.21,1.81,0.21c3.08,0,5.97-1.75,7.35-4.63c81.79-170.48,158.11-233.12,289.03-235.58 V522C438.47,525.19,440.34,528.09,443.25,529.42z M151.83,607.06c15.41-182.94,141.74-326.08,294.8-326.08 c4.51,0,8.16-3.65,8.16-8.16v-99.67l190.67,165.48L454.79,504.11v-99.67c0-4.51-3.65-8.16-8.16-8.16 C314.35,396.29,231.89,454.14,151.83,607.06z" /> </svg> </div> - <div className="text-white">Share</div> + <span className="text-white">Share</span> </Button>
🧹 Nitpick comments (1)
apps/frontend/src/components/launches/providers/high.order.provider.tsx (1)
235-258: Enhance error handling in handleShare functionThe implementation looks good and includes proper safety checks. Consider being more specific with error handling by checking for specific error types.
} catch (err) { - if (err instanceof Error) - toast.show(`Failed to copy the link. ${err.message}`, 'warning'); + if (err instanceof Error) { + const message = err.name === 'NotAllowedError' + ? 'Permission denied. Please ensure clipboard access is allowed.' + : err.message; + toast.show(`Failed to copy the link. ${message}`, 'warning'); + } else { + toast.show('Failed to copy the link. Unknown error occurred.', 'warning'); + } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/frontend/src/components/launches/add.edit.model.tsx(3 hunks)apps/frontend/src/components/launches/calendar.tsx(3 hunks)apps/frontend/src/components/launches/providers/high.order.provider.tsx(8 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/frontend/src/components/launches/calendar.tsx
- apps/frontend/src/components/launches/add.edit.model.tsx
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/frontend/src/components/launches/providers/high.order.provider.tsx (1)
272-294: Enhance error handling and accessibility in share functionality.The implementation is good but could be improved:
- Add keyboard support for the share button
- Consider adding a success message with the copied URL
const handleShare = async () => { if (!existingData.posts.length) { return toast.show('No posts available to share', 'warning'); } const postId = existingData.posts[0].id; const previewPath = new URL( `/preview/${postId}`, window.location.origin ).toString(); try { if (!navigator.clipboard) { throw new Error('Clipboard API not available'); } await navigator.clipboard.writeText(previewPath); - return toast.show('Link copied to clipboard.', 'success'); + return toast.show(`Link copied to clipboard: ${previewPath}`, 'success'); } catch (err) { if (err instanceof Error) toast.show(`Failed to copy the link. ${err.message}`, 'warning'); } };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/frontend/src/components/launches/add.edit.model.tsx(3 hunks)apps/frontend/src/components/launches/calendar.tsx(3 hunks)apps/frontend/src/components/launches/providers/high.order.provider.tsx(7 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/frontend/src/components/launches/calendar.tsx
- apps/frontend/src/components/launches/add.edit.model.tsx
🧰 Additional context used
🪛 Biome (1.9.4)
apps/frontend/src/components/launches/providers/high.order.provider.tsx
[error] 297-297: Shouldn't redeclare 'changeToEditor'. Consider to delete it or rename it.
'changeToEditor' is defined here:
(lint/suspicious/noRedeclare)
[error] 441-441: Unexpected token. Did you mean {'}'} or }?
(parse)
[error] 459-459: Expected corresponding JSX closing tag for 'EditorWrapper'.
Opening tag
closing tag
(parse)
[error] 616-616: expected ) but instead found className
Remove className
(parse)
[error] 619-619: expected , but instead found onClick
Remove onClick
(parse)
[error] 620-620: expected , but instead found className
Remove className
(parse)
[error] 638-638: expected , but instead found className
Remove className
(parse)
[error] 638-638: expected , but instead found <
Remove <
(parse)
[error] 639-639: expected , but instead found <
Remove <
(parse)
[error] 641-641: expected , but instead found <
Remove <
(parse)
[error] 642-642: expected } but instead found )
Remove )
(parse)
[error] 642-642: Unexpected token. Did you mean {'}'} or }?
(parse)
[error] 647-647: expected , but instead found {
Remove {
(parse)
[error] 647-647: Expected a property, a shorthand property, a getter, a setter, or a method but instead found '('.
Expected a property, a shorthand property, a getter, a setter, or a method here.
(parse)
[error] 647-647: expected , but instead found ===
Remove ===
(parse)
[error] 647-647: expected , but instead found showTab
Remove showTab
(parse)
[error] 647-647: expected , but instead found ===
Remove ===
(parse)
[error] 647-647: Expected a function body but instead found '&&'.
Expected a function body here.
(parse)
[error] 648-648: expected , but instead found <
Remove <
(parse)
[error] 648-648: expected , but instead found className
Remove className
(parse)
[error] 648-648: Expected a parameter but instead found ''mt-[20px]''.
Expected a parameter here.
(parse)
[error] 648-648: expected , but instead found !==
Remove !==
(parse)
[error] 649-649: Expected a parenthesis '(' but instead found '<'.
Expected a parenthesis '(' here.
(parse)
[error] 649-649: expected , but instead found values
Remove values
(parse)
[error] 649-649: ';' expected'
An explicit or implicit semicolon is expected here...
(parse)
[error] 649-649: expected , but instead found /
Remove /
(parse)
[error] 650-650: Expected a parenthesis '(' but instead found '{'.
Expected a parenthesis '(' here.
(parse)
[error] 616-618: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.
(lint/suspicious/noAssignInExpressions)
[error] 619-619: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.
(lint/suspicious/noAssignInExpressions)
[error] 620-638: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.
(lint/suspicious/noAssignInExpressions)
[error] 638-638: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.
(lint/suspicious/noAssignInExpressions)
[error] 647-647: Duplicate parameter name.
The parameter overrides a preceding parameter by using the same name.
(lint/suspicious/noDuplicateParameters)
🔇 Additional comments (1)
apps/frontend/src/components/launches/providers/high.order.provider.tsx (1)
618-639: 🛠️ Refactor suggestionImprove Share button accessibility.
The Share button needs accessibility enhancements:
<Button onClick={handleShare} className="!h-[24px] rounded-[3px] flex gap-[4px] w-[102px] text-[12px] font-[500]" + aria-label="Share post" + role="button" > <div> <svg xmlns="http://www.w3.org/2000/svg" version="1.1" - x="0px" - y="0px" viewBox="0 0 800 1000" className="max-w-[20px] max-h-[20px] fill-current h-[1.25em]" + aria-hidden="true" + role="img" > <path fill="white" d="M443.25,529.42c2.9,1.32,6.32,0.83,8.73-1.27l211.28-183.37c1.79-1.55,2.81-3.79,2.81-6.16c0-2.37-1.02-4.61-2.81-6.16 L451.98,149.11c-2.41-2.1-5.83-2.59-8.73-1.27c-2.91,1.33-4.77,4.23-4.77,7.42v109.52c-168.67,5.27-304.54,173.69-304.54,379.94 c0,3.81,2.63,7.11,6.35,7.95c0.61,0.14,1.21,0.21,1.81,0.21c3.08,0,5.97-1.75,7.35-4.63c81.79-170.48,158.11-233.12,289.03-235.58V522C438.47,525.19,440.34,528.09,443.25,529.42z M151.83,607.06c15.41-182.94,141.74-326.08,294.8-326.08 c4.51,0,8.16-3.65,8.16-8.16v-99.67l190.67,165.48L454.79,504.11v-99.67c0-4.51-3.65-8.16-8.16-8.16 C314.35,396.29,231.89,454.14,151.83,607.06z" /> </svg> </div> - <div className="text-white">Share</div> + <span className="text-white">Share</span> </Button>Likely invalid or redundant comment.
🧰 Tools
🪛 Biome (1.9.4)
[error] 619-619: expected
,but instead foundonClickRemove onClick
(parse)
[error] 620-620: expected
,but instead foundclassNameRemove className
(parse)
[error] 638-638: expected
,but instead foundclassNameRemove className
(parse)
[error] 638-638: expected
,but instead found<Remove <
(parse)
[error] 639-639: expected
,but instead found<Remove <
(parse)
[error] 619-619: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.(lint/suspicious/noAssignInExpressions)
[error] 620-638: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.(lint/suspicious/noAssignInExpressions)
[error] 638-638: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.(lint/suspicious/noAssignInExpressions)
apps/frontend/src/components/launches/providers/high.order.provider.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
apps/frontend/src/components/launches/providers/high.order.provider.tsx (3)
459-480: Improve Share button accessibility.The Share button lacks proper accessibility attributes and has unnecessary SVG attributes.
Apply this diff to enhance accessibility:
<Button onClick={handleShare} className="!h-[24px] rounded-[3px] flex gap-[4px] w-[102px] text-[12px] font-[500]" + aria-label="Share post" + role="button" > <div> <svg xmlns="http://www.w3.org/2000/svg" version="1.1" - x="0px" - y="0px" viewBox="0 0 800 1000" className="max-w-[20px] max-h-[20px] fill-current h-[1.25em]" + aria-hidden="true" + role="img" > <path fill="white" d="M443.25,529.42c2.9,1.32,6.32,0.83,8.73-1.27l211.28-183.37c1.79-1.55,2.81-3.79,2.81-6.16c0-2.37-1.02-4.61-2.81-6.16 L451.98,149.11c-2.41-2.1-5.83-2.59-8.73-1.27c-2.91,1.33-4.77,4.23-4.77,7.42v109.52c-168.67,5.27-304.54,173.69-304.54,379.94 c0,3.81,2.63,7.11,6.35,7.95c0.61,0.14,1.21,0.21,1.81,0.21c3.08,0,5.97-1.75,7.35-4.63c81.79-170.48,158.11-233.12,289.03-235.58 V522C438.47,525.19,440.34,528.09,443.25,529.42z M151.83,607.06c15.41-182.94,141.74-326.08,294.8-326.08 c4.51,0,8.16-3.65,8.16-8.16v-99.67l190.67,165.48L454.79,504.11v-99.67c0-4.51-3.65-8.16-8.16-8.16 C314.35,396.29,231.89,454.14,151.83,607.06z" /> </svg> </div> - <div className="text-white">Share</div> + <span className="text-white">Share</span> </Button>
401-405: Consider using a constant for minimum content length.The minimum content length check is hardcoded. Consider extracting it to a named constant for better maintainability.
+const MIN_CONTENT_LENGTH = 6; + {(!val.content || val.content.length < 6) && ( <div className="my-[5px] text-customColor19 text-[12px] font-[500]"> - The post should be at least 6 characters long + The post should be at least {MIN_CONTENT_LENGTH} characters long </div> )}
254-276: SimplifychangeToEditorfunction.The function contains duplicate confirmation dialog text and could be simplified.
const changeToEditor = useCallback(async () => { + const confirmMessage = !editInPlace + ? 'Are you sure you want to edit only this?' + : 'Are you sure you want to revert it back to global editing?'; + if ( - !(await deleteDialog( - !editInPlace - ? 'Are you sure you want to edit only this?' - : 'Are you sure you want to revert it back to global editing?', - 'Yes, edit in place!' - )) + !(await deleteDialog(confirmMessage, 'Yes, edit in place!')) ) { return false; } setEditInPlace(!editInPlace); setInPlaceValue( editInPlace ? [{ content: '' }] : props.value.map((p) => ({ id: p.id, content: p.content, image: p.image, })) ); }, [props.value, editInPlace]);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/frontend/src/components/launches/providers/high.order.provider.tsx(2 hunks)
🔇 Additional comments (2)
apps/frontend/src/components/launches/providers/high.order.provider.tsx (2)
229-251:⚠️ Potential issueEnhance error handling in share functionality.
The share functionality has several potential issues:
- No type checking for
existingData.posts[0].id- No validation of the generated URL
- Generic error handling could expose internal details
Apply this diff to improve error handling:
const handleShare = async () => { - if (!existingData.posts.length) { + if (!existingData?.posts?.length) { return toast.show('No posts available to share', 'warning'); } const postId = existingData.posts[0].id; + if (!postId) { + return toast.show('Invalid post ID', 'warning'); + } const previewPath = new URL( `/preview/${postId}`, window.location.origin ).toString(); try { if (!navigator.clipboard) { throw new Error('Clipboard API not available'); } await navigator.clipboard.writeText(previewPath); return toast.show('Link copied to clipboard.', 'success'); } catch (err) { - if (err instanceof Error) - toast.show(`Failed to copy the link. ${err.message}`, 'warning'); + toast.show('Failed to copy the link to clipboard', 'warning'); + console.error('Share error:', err); } };Likely invalid or redundant comment.
89-89: 🛠️ Refactor suggestionConsider making
idrequired for post objects.The
idproperty is marked as optional but appears to be required for the share functionality. This could lead to runtime errors if posts without IDs are processed.- id?: string; + id: string;✅ Verification successful
Making
idrequired is the correct approachThe share functionality explicitly requires a post ID to generate preview URLs and there's no fallback mechanism for posts without IDs. Making the field required would prevent potential runtime errors and better reflect its actual usage in the codebase.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if there are any posts without IDs in the codebase rg -A 5 "content:\s*['\"]" | rg -v "id:"Length of output: 65579
Script:
#!/bin/bash # Search for share-related functionality and post ID usage echo "=== Share functionality implementation ===" ast-grep --pattern 'handleShare($$$)' -l typescript echo -e "\n=== Post object creation patterns ===" ast-grep --pattern 'new Post($$$)' -l typescript ast-grep --pattern '{ content: $_, id: $_ }' -l typescript echo -e "\n=== ID usage in share operations ===" rg -l "share.*id" --type tsLength of output: 470
Script:
#!/bin/bash # Search with broader patterns and include context echo "=== Share-related code ===" rg -A 5 "share" --type ts echo -e "\n=== Post type definitions ===" rg -A 5 "type.*Post.*=|interface.*Post.*{" --type ts echo -e "\n=== ID usage in component ===" rg -A 5 "id.*:" "apps/frontend/src/components/launches/providers/high.order.provider.tsx"Length of output: 6993
| ))} | ||
| </div> | ||
| </EditorWrapper>, | ||
| document.querySelector('#renderEditor')! |
There was a problem hiding this comment.
💡 Codebase verification
The non-null assertion on querySelector('#renderEditor') is unsafe and should be fixed.
The element is defined in a different component (add.edit.model.tsx) and its existence depends on component mounting order and conditional rendering. The suggested fallback to document.body provides a safe default target.
- The element is conditionally rendered with a 'hidden' class in add.edit.model.tsx
- No guarantees exist about component lifecycle ordering
- Runtime errors are possible if the target element isn't mounted when the querySelector runs
🔗 Analysis chain
Verify DOM element existence.
The code assumes #renderEditor element exists in the DOM. This could cause runtime errors if the element is not present.
Add a safety check:
- document.querySelector('#renderEditor')!
+ document.querySelector('#renderEditor') || document.body🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for #renderEditor element definition
rg -l "id=['\"]renderEditor['\"]"
Length of output: 96
Script:
#!/bin/bash
# Get the element definition context
rg -A 5 -B 5 'id=["\']renderEditor["\']' apps/frontend/src/components/launches/add.edit.model.tsx
# Check if high.order.provider is imported in add.edit.model
rg "from ['\"].*high\.order\.provider" apps/frontend/src/components/launches/add.edit.model.tsx
# Check if add.edit.model is imported in high.order.provider
rg "from ['\"].*add\.edit\.model" apps/frontend/src/components/launches/providers/high.order.provider.tsx
Length of output: 116
Script:
#!/bin/bash
# Get the element definition context
rg -A 5 -B 5 "renderEditor" apps/frontend/src/components/launches/add.edit.model.tsx
# Get the querySelector context
rg -A 5 -B 5 "querySelector.*renderEditor" apps/frontend/src/components/launches/providers/high.order.provider.tsx
# Check component relationships
rg "high\.order\.provider" apps/frontend/src/components/launches/add.edit.model.tsx
rg "add\.edit\.model" apps/frontend/src/components/launches/providers/high.order.provider.tsx
Length of output: 1165
| return [...prev]; | ||
| }); | ||
| }, | ||
| [InPlaceValue] |
Check warning
Code scanning / ESLint
verifies the list of dependencies for Hooks like useEffect and similar
| <Fragment key={`edit_inner_${index}`}> | ||
| <div> | ||
| <div className="flex gap-[4px]"> | ||
| <div className="flex-1 text-textColor editor"> |
Check warning
Code scanning / ESLint
Disallow non-null assertions using the `!` postfix operator
| <div> | ||
| <div className="flex gap-[4px]"> | ||
| <div className="flex-1 text-textColor editor"> | ||
| {integration?.identifier === 'linkedin' && ( |
Check warning
Code scanning / ESLint
Disallow non-null assertions using the `!` postfix operator
|
@nevo-david Can you Review this? |
|
Your PR's base branch was set to |
|
Looks like the Postiz-CI/CD Check failed, could you have a look at it? |
4 participants
What kind of change does this PR introduce?
Created a post preview page
Why was this change needed?
Related #218
Other information:
I am open to discuss more about this feature
Checklist:
Put a "X" in the boxes below to indicate you have followed the checklist;
https://www.loom.com/share/ef80c01856314528a1a79ba0be3d72d7?sid=ea1248bf-b1d7-46fe-b5a8-036b102b65a8
Summary by CodeRabbit
New Features
Previewcomponent for fetching and displaying posts.GeneralPreviewComponentwith visual indicators for truncated text.Bug Fixes
AddEditModal.Style
Chores