feat: Add social sharing buttons and OpenGraph meta tags #120
Conversation
|
thank you for the contribution and for including a preview screenshot as well! We will take a look at the code soon. |
|
GM @SurfingBowser |
There was a problem hiding this comment.
Pull request overview
This PR adds social media sharing capabilities and SEO optimization with OpenGraph meta tags to enhance content discoverability and shareability. It introduces two new reusable components for managing SEO metadata and social sharing buttons, integrating them into the Vulnerability Details and Report Details pages.
Changes:
- Added
SeoHeadcomponent for managing OpenGraph and Twitter Card meta tags - Added
ShareButtonscomponent with X (Twitter), LinkedIn, and Copy Link functionality - Integrated both components into Vulnerability Details and Report Details pages
- Wrapped the application in
HelmetProviderto enable dynamic head tag management
Reviewed changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| UI/src/main.tsx | Wrapped app root in HelmetProvider to support dynamic meta tag management |
| UI/src/features/pages/regular/vulnerability-details/vulnerability-details.tsx | Added SeoHead and ShareButtons components to vulnerability detail view |
| UI/src/features/pages/regular/report-details/report-details.tsx | Added SeoHead and ShareButtons components to report detail view |
| UI/src/components/common/ShareButtons.tsx | New component providing social media sharing buttons with clipboard copy functionality |
| UI/src/components/common/SeoHead.tsx | New component for managing SEO meta tags including OpenGraph and Twitter Cards |
| UI/package.json | Added react-helmet-async dependency for meta tag management |
Files not reviewed (1)
- UI/package-lock.json: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| <SeoHead | ||
| title={vulnerability.title} | ||
| description={vulnerability.description?.substring(0, 160)} | ||
| url={window.location.href} | ||
| /> |
There was a problem hiding this comment.
The SeoHead component is placed inside the page content rather than in the document head section. This component should be moved outside the main content Box, ideally near the top of the component's return statement, to ensure meta tags are properly rendered in the document .
UI/src/features/pages/regular/report-details/report-details.tsx
Outdated
Show resolved
Hide resolved
| navigator.clipboard.writeText(shareUrl).then(() => { | ||
| setSnackbarOpen(true); | ||
| }); |
There was a problem hiding this comment.
The clipboard write operation lacks error handling. If the copy fails (e.g., due to permissions or browser compatibility), the user receives no feedback. Add a .catch() block to handle failures and show an appropriate error message to the user.
|
@codebestia please restore UI/package-lock.json and take a look at the co-pilot suggestions please |
|
@SurfingBowser |
|
@SurfingBowser @0xGeorgii |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| {vulnerability && ( | ||
| <SeoHead | ||
| title={vulnerability.title} | ||
| description={vulnerability.description?.substring(0, 160) ?? ""} |
There was a problem hiding this comment.
The magic number 160 should be extracted as a named constant (e.g., MAX_DESCRIPTION_LENGTH) to improve code maintainability and make its purpose clear.
UI/src/components/common/SeoHead.tsx
Outdated
| const metaDescription = description || 'Soroban security portal - audits, reports, and vulnerabilities.'; | ||
| const metaUrl = url || window.location.href; | ||
| // Fallback image could be a generic logo if not provided | ||
| const metaImage = image || `${window.location.origin}/static/images/logo.png`; |
There was a problem hiding this comment.
The hardcoded image path '/static/images/logo.png' should be extracted as a constant, preferably in a configuration file, to make it easier to update and maintain.
| console.error('Failed to copy link'); | ||
| alert('Failed to copy link'); |
There was a problem hiding this comment.
Using window.alert() for error handling provides poor user experience. Consider using the existing Snackbar component with severity='error' to display the error message consistently with the success case.
|
@0xGeorgii co-pilot review implemented |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const handleCopyLink = () => { | ||
| navigator.clipboard.writeText(shareUrl).then(() => { | ||
| setSnackbarOpen(true); | ||
| }).catch(() => { | ||
| console.error('Failed to copy link'); | ||
| setSnackbarOpen(true); | ||
| setShareError(true); | ||
| }); | ||
| }; |
There was a problem hiding this comment.
The shareError state is not reset when successfully copying a link. If a user encounters an error and then successfully copies a link, the success message will incorrectly show as an error. Add setShareError(false) in the success handler before setSnackbarOpen(true).
There was a problem hiding this comment.
@codebestia, please address this comment. I think it makes sense
Co-authored-by: Copilot <[email protected]>
3 participants
Description
This PR introduces social media sharing capabilities and SEO with OpenGraph for key pages in the application.
Changes
New Components
Integration
SeoHeadandShareButtonsto Vulnerability Details and Report Details pages.Screenshots
Related Issue
Closes #99