-
Notifications
You must be signed in to change notification settings - Fork 29
feat(article): add social share buttons with all clickable link for all sharing options and pre-filled content #62
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
base: main
Are you sure you want to change the base?
Conversation
WalkthroughAdded client-side social sharing UI to the blog post client: new Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ 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 |
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.
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
♻️ Duplicate comments (1)
app/a/[slug]/BlogPostClient.tsx (1)
78-149: (Duplicate) Share endpoints/params may be ignored/brittle—consider validating against current platform docs.
This is the same concern as earlier review notes (LinkedInshare-offsiteextra params; Facebookquote; X intent params).Verify current (Dec 2025) query parameters supported for: - https://twitter.com/intent/tweet - https://www.linkedin.com/sharing/share-offsite/ - https://www.facebook.com/sharer/sharer.php Specifically whether LinkedIn accepts title/summary and whether Facebook accepts quote.
🧹 Nitpick comments (2)
app/a/[slug]/BlogPostClient.tsx (2)
3-13:copiedstate is fine, but you’ll want a timeout cleanup hook for the latersetTimeout.
Right now the timeout is created in the click handler (Line 157); add a ref + cleanup so you don’t fire state updates after unmount / stack multiple timers.
150-182: Harden “Copy Link”: secure-context/feature detect + dedupe/cleanup the timer.
This avoids failures on non-HTTPS contexts, avoids stacking multiple timeouts on repeated clicks, and avoids a timer firing after unmount.import { useState } from "react" +import { useEffect, useRef } from "react" export default function BlogPostPage({ post }: { post: BlogPost }) { const [copied, setCopied] = useState(false) + const copiedTimeoutRef = useRef<number | null>(null) + + useEffect(() => { + return () => { + if (copiedTimeoutRef.current != null) window.clearTimeout(copiedTimeoutRef.current) + } + }, []) return ( @@ <button type="button" onClick={() => { - navigator.clipboard.writeText(window.location.href) + if (!window.isSecureContext || !navigator.clipboard?.writeText) { + alert('Copy is only available on HTTPS. Please copy the URL from the address bar.') + return + } + navigator.clipboard.writeText(window.location.href) .then(() => { + if (copiedTimeoutRef.current != null) window.clearTimeout(copiedTimeoutRef.current) setCopied(true) - setTimeout(() => setCopied(false), 2000) + copiedTimeoutRef.current = window.setTimeout(() => setCopied(false), 2000) }) .catch((err) => { console.error('Failed to copy:', err) alert('Failed to copy link. Please copy manually.') }) }}
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: 0
♻️ Duplicate comments (1)
app/a/[slug]/BlogPostClient.tsx (1)
104-151: LinkedIn/Facebook share params may not do what you expect.
share-offsiteoften ignorestitle/summary, and Facebook sharing generally relies on OG tags (thequoteparam may be ignored). (This was raised in earlier reviews.)
🧹 Nitpick comments (2)
app/a/[slug]/BlogPostClient.tsx (2)
76-151: Share popup implementation is OK; consider a Web Share API fast-path.
On mobile,navigator.share()(when available) is typically a better UX than popups; you can keepwindow.openas fallback.
153-190: Copy Link flow is robust; tweak secure-context messaging + a11y announcement.
- Message: “secure context (HTTPS or localhost)” is more accurate than “HTTPS only”.
- A11y: consider adding a small
role="status"/aria-live="polite"region so screen readers get “Copied!” feedback without relying on visual change.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/a/[slug]/BlogPostClient.tsx(4 hunks)
🔇 Additional comments (2)
app/a/[slug]/BlogPostClient.tsx (2)
3-19: Timeout cleanup + state approach looks solid (no obvious leaks / stale timers).
Only nit: consider tightening the cleanup block style (braces/newlines) for readability, but functionally LGTM.
40-48:<Image src={post.image}>simplification is good — less branching, same behavior.
Just double-checkpost.imageis always a valuenext/imagecan render in your deployment (relative path vs remote URL expectations).
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: 0
🧹 Nitpick comments (1)
app/globals.css (1)
13-25: Solid.sr-onlyutility for accessibility feedback.The screen-reader-only utility follows standard accessibility patterns and is correctly placed in the
@layer utilitiessection. This supports the copy-to-clipboard announcement feature referenced in the PR.Optional modernization note: The
clip: rect(0, 0, 0, 0)property works reliably across browsers, but consider the more explicit modern approachclip-path: inset(50%)if you update accessibility utilities in the future.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
app/a/[slug]/BlogPostClient.tsx(4 hunks)app/globals.css(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- app/a/[slug]/BlogPostClient.tsx
🔇 Additional comments (1)
app/globals.css (1)
1-107: Note: Missing primary implementation file.The PR description indicates changes to
app/a/[slug]/BlogPostClient.tsx(+95, -6 lines) that implements the social share buttons, but this file is not provided for review. The changes toapp/globals.cssappear sound, but a comprehensive review of the social share feature (button logic, accessibility, responsive layout, clipboard handling) requires reviewing the main component file.If you'd like a complete review of the social share feature, please provide the
app/a/[slug]/BlogPostClient.tsxfile.
02161ad3-b35c-4282-8454-d6efb814aac6.mp4
Problem
Fixes #53
The article pages lacked social sharing functionality, making it difficult for readers to share content on social media platforms. This reduced content discoverability and user engagement.
Solution
Added social share buttons to individual article pages with the following features:
Share Platforms
Implementation Details
flex-wrapfor mobile devicesaria-labelattributes for accessibilityBlogPostClient.tsx)Technical Approach
window.open()for social media share dialogsnavigator.clipboard.writeText()for copy functionalityChanges Made
app/a/[slug]/BlogPostClient.tsx(+95 lines, -6 lines)Testing
✅ Dev server runs without errors
✅ All four share buttons display correctly
✅ Buttons maintain consistent styling across devices
✅ Responsive layout works on mobile and desktop
✅ No regressions in existing functionality
Screenshots
See attached screenshots in the PR showing:
Summary by CodeRabbit
New Features
Accessibility
Improvements
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.