Add ShareButton component #5406
Conversation
tindevw
added
review-needed
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Skipped Deployment
|
|
I rebased this for you because my vite PR created a bunch of merge conflicts. To start working locally again just do |
| {loggedIn && ( | ||
| <ShareButton | ||
| title={event?.title || 'Sjekk ut dette arrangementet!'} | ||
| url={`${appConfig?.webUrl}/events/${event?.id}`} | ||
| /> | ||
| )} |
There was a problem hiding this comment.
I suggest moving this down to comments and reactions, as I feel that's common. The sidebar should really only contain information and details regarding the event, while the bottom section contains "user interactions".
There was a problem hiding this comment.
Looks a bit out of place
This is before:
There was a problem hiding this comment.
I like having it in the sidebar, but an alternative might be to the right of "Kommentarer":
|
Updated changes: Events: Joblisting: Which icon looks better?
|
Co-authored-by: viljeno <[email protected]>
Co-authored-by: viljeno <[email protected]>
tindevw
force-pushed
the
add-ShareButton
branch
from
d014588 to
fdce069
Compare
📝 WalkthroughWalkthroughThis pull request introduces a new Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant SB as ShareButton Component
participant API as Web APIs
U->>SB: Click Share Button
SB->>API: Check if navigator.canShare(url)
alt Web Share API Supported
SB->>API: Call navigator.share({ title, url })
else API Not Supported
SB->>API: Call navigator.clipboard.writeText(url)
end
SB->>SB: Dispatch toast notification
Tip ⚡🧪 Multi-step agentic review comment chat (experimental)
✨ Finishing Touches
🪧 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: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
lego-webapp/components/ShareButton/index.tsx(1 hunks)lego-webapp/pages/events/@eventIdOrSlug/+Page.tsx(2 hunks)lego-webapp/pages/joblistings/@joblistingIdOrSlug/+Page.tsx(2 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
lego-webapp/components/ShareButton/index.tsx
[error] 17-19: This block statement doesn't serve any purpose and can be safely removed.
Standalone block statements without any block-level declarations are redundant in JavaScript and can be removed to simplify the code.
Safe fix: Remove redundant block.
(lint/complexity/noUselessLoneBlockStatements)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (4)
lego-webapp/pages/joblistings/@joblistingIdOrSlug/+Page.tsx (1)
240-243: ShareButton implementation looks good.The ShareButton is appropriately placed in the sidebar with a clear title and correctly constructs the URL using the job listing's ID.
lego-webapp/pages/events/@eventIdOrSlug/+Page.tsx (1)
312-315: ShareButton implementation looks good.The ShareButton is appropriately placed in the sidebar after a divider line. The title is clear and the URL is correctly constructed using the event's ID.
lego-webapp/components/ShareButton/index.tsx (2)
6-9: Props type definition looks good.The Props interface clearly defines the required properties for the ShareButton component.
29-36: Button implementation is correct.The button is well-structured with appropriate layout, icon, and text. Previous reviewer comments about using only an icon were addressed by retaining the text as requested.
| const handleShare = async () => { | ||
| if (navigator?.canShare?.({ title, url })) { | ||
| navigator.share({ title, url }); | ||
| } else { | ||
| { | ||
| navigator.clipboard.writeText(url); | ||
| } | ||
| dispatch( | ||
| addToast({ | ||
| message: 'Link er kopiert til utklippstavlen', | ||
| type: 'success', | ||
| }), | ||
| ); | ||
| } | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fix unnecessary block statement and add error handling.
The share functionality works as expected, but there's an unnecessary block statement and error handling should be added for clipboard and share operations.
const handleShare = async () => {
if (navigator?.canShare?.({ title, url })) {
- navigator.share({ title, url });
+ try {
+ await navigator.share({ title, url });
+ } catch (error) {
+ console.error('Error sharing:', error);
+ await copyToClipboard();
+ }
} else {
- {
- navigator.clipboard.writeText(url);
- }
- dispatch(
- addToast({
- message: 'Link er kopiert til utklippstavlen',
- type: 'success',
- }),
- );
+ await copyToClipboard();
+ }
+};
+
+const copyToClipboard = async () => {
+ try {
+ await navigator.clipboard.writeText(url);
+ dispatch(
+ addToast({
+ message: 'Link er kopiert til utklippstavlen',
+ type: 'success',
+ }),
+ );
+ } catch (error) {
+ console.error('Error copying to clipboard:', error);
+ dispatch(
+ addToast({
+ message: 'Kunne ikke kopiere link',
+ type: 'error',
+ }),
+ );
}
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const handleShare = async () => { | |
| if (navigator?.canShare?.({ title, url })) { | |
| navigator.share({ title, url }); | |
| } else { | |
| { | |
| navigator.clipboard.writeText(url); | |
| } | |
| dispatch( | |
| addToast({ | |
| message: 'Link er kopiert til utklippstavlen', | |
| type: 'success', | |
| }), | |
| ); | |
| } | |
| }; | |
| const handleShare = async () => { | |
| if (navigator?.canShare?.({ title, url })) { | |
| try { | |
| await navigator.share({ title, url }); | |
| } catch (error) { | |
| console.error('Error sharing:', error); | |
| await copyToClipboard(); | |
| } | |
| } else { | |
| await copyToClipboard(); | |
| } | |
| }; | |
| const copyToClipboard = async () => { | |
| try { | |
| await navigator.clipboard.writeText(url); | |
| dispatch( | |
| addToast({ | |
| message: 'Link er kopiert til utklippstavlen', | |
| type: 'success', | |
| }), | |
| ); | |
| } catch (error) { | |
| console.error('Error copying to clipboard:', error); | |
| dispatch( | |
| addToast({ | |
| message: 'Kunne ikke kopiere link', | |
| type: 'error', | |
| }), | |
| ); | |
| } | |
| }; |
🧰 Tools
🪛 Biome (1.9.4)
[error] 17-19: This block statement doesn't serve any purpose and can be safely removed.
Standalone block statements without any block-level declarations are redundant in JavaScript and can be removed to simplify the code.
Safe fix: Remove redundant block.
(lint/complexity/noUselessLoneBlockStatements)
| if (navigator?.canShare?.({ title, url })) { | ||
| navigator.share({ title, url }); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Consider browser compatibility for Web Share API.
While you're correctly checking for navigator.canShare support, some browsers might partially implement the API but have limitations. Consider adding a fallback for unsupported share data types.
🌐 Web query:
Which browsers support the Web Share API and what are their limitations?
💡 Result:
The Web Share API is supported across most modern browsers, but with varying levels of compatibility and limitations. Here's a detailed breakdown:
Browser Support
-
Chrome:
- ✅ Full support: v89+ (desktop), all versions (Android)
- ◐ Partial support: v89–v131 (desktop)
- ❌ No support: v4–v88 [1][4]
-
Edge:
- ✅ Full support: v95+
- ◐ Partial support: v81–94
- ❌ No support: v12–80 [1][4][10]
-
Safari:
- ✅ Full support: v12.1+ (macOS), v12.2+ (iOS)
- ❌ No support: older versions [1][4][7]
-
Opera:
- ✅ Full support: v114+ (desktop), v72–80 (Mobile)
- ❌ No support: v9.5–113 [1][4]
-
Mobile Browsers:
- ✅ Chrome for Android: All versions
- ✅ Firefox for Android: All versions
- ✅ Samsung Internet: v8.2+ [1][4]
| Browser | Desktop Support | Mobile Support |
|---|---|---|
| Chrome | v89+ (partial/full) | All versions (full) |
| Edge | v95+ (full) | Not specified |
| Safari | v12.1+ (macOS) | v12.2+ (iOS) |
| Firefox | ❌ | ✅ All (Android) |
| Opera | v114+ (full) | v72–80 (Mobile) |
Key Limitations
-
Secure Context Requirement:
- Requires HTTPS in production. Localhost/testing environments may not work unless using tools like JSFiddle [3][7][10].
-
Desktop Browser Gaps:
- Poor desktop support except Chrome and Edge (partial/full). Firefox desktop lacks support entirely [3][4][7].
-
User Interaction Dependency:
- Must be triggered by transient activation (e.g., button click). Background/script-initiated sharing fails [8][10].
-
Variable Share Targets:
- Available apps depend on the OS/device. Developers can’t customize the list or track which app was used [3][7].
-
Third-Party Iframe Restrictions:
- Embedded iframes need explicit permissions via
<iframe allow="web-share">. Otherwise,NotAllowedErroroccurs [5][6].
- Embedded iframes need explicit permissions via
-
File Sharing Constraints:
- Requires validation via
navigator.canShare({ files })first. Large files or unsupported types may fail silently [7][12].
- Requires validation via
For unsupported browsers (e.g., Firefox desktop), developers must implement fallback sharing UIs. Compatibility checks with navigator.canShare() are recommended before invoking the API [7][10].
Citations:
- 1: https://www.lambdatest.com/web-technologies/web-share
- 2: https://www.w3.org/TR/web-share/
- 3: https://sudolabs.com/insights/share-visual-content-from-web-to-social-media-without-api-or-sdk
- 4: https://caniuse.com/web-share
- 5: https://developer.chrome.com/blog/web-share-api-in-third-party-iframes
- 6: https://w3c.github.io/web-share/
- 7: https://www.telerik.com/blogs/definitive-guide-using-web-share-api
- 8: https://developer.mozilla.org/en-US/docs/Web/API/Navigator/share
- 9: https://apidocs.webshare.io
- 10: https://developer.mozilla.org/en-US/docs/Web/API/Web_Share_API
- 11: https://apidocs.webshare.io/verification/limits
- 12: https://www.raymondcamden.com/2023/04/20/testing-the-web-share-api
- 13: https://www.lambdatest.com/web-share-api
Web Share API: Enhance Fallbacks for Partial & Unsupported Environments
In lego-webapp/components/ShareButton/index.tsx (lines 14–15), you’re correctly checking for the existence of navigator?.canShare before calling navigator.share. However, as our latest research indicates, browser support for the Web Share API is nuanced:
-
Variability Across Browsers:
- Chrome & Edge: Generally provide full or partial support, though you might encounter differences based on the version.
- Safari: Supports the API on modern macOS and iOS versions.
- Firefox Desktop: Does not support the API at all, so users on Firefox will not be able to share via this mechanism.
-
Limitations & Fallback Needs:
- Even when
navigator.canShareis available, certain share data types (especially files) may not be fully supported. - The API requires a secure context (HTTPS) and user interaction, which might not cover all scenarios.
- A fallback mechanism—such as a copy-to-clipboard for URLs or a custom sharing modal—can help address these discrepancies and improve the user experience in unsupported or partially supported environments.
- Even when
Please consider implementing a graceful fallback for cases where the API’s capabilities are limited or absent. This will ensure a more consistent sharing experience across all browsers.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
lego-webapp/components/ShareButton/index.tsx (1)
13-27:⚠️ Potential issueRefactor sharing functionality with proper error handling.
The current implementation has several issues:
- The unnecessary block statement around the clipboard operation
- Lack of error handling for share and clipboard operations
- Not properly awaiting async operations
This was previously identified in a past review but hasn't been addressed.
const handleShare = async () => { if (navigator?.canShare?.({ title, url })) { - navigator.share({ title, url }); + try { + await navigator.share({ title, url }); + } catch (error) { + console.error('Error sharing:', error); + await copyToClipboard(); + } } else { - { - navigator.clipboard.writeText(url); - } - dispatch( - addToast({ - message: 'Link er kopiert til utklippstavlen', - type: 'success', - }), - ); + await copyToClipboard(); + } +}; + +const copyToClipboard = async () => { + try { + await navigator.clipboard.writeText(url); + dispatch( + addToast({ + message: 'Link er kopiert til utklippstavlen', + type: 'success', + }), + ); + } catch (error) { + console.error('Error copying to clipboard:', error); + dispatch( + addToast({ + message: 'Kunne ikke kopiere link', + type: 'error', + }), + ); } };🧰 Tools
🪛 Biome (1.9.4)
[error] 17-19: This block statement doesn't serve any purpose and can be safely removed.
Standalone block statements without any block-level declarations are redundant in JavaScript and can be removed to simplify the code.
Safe fix: Remove redundant block.(lint/complexity/noUselessLoneBlockStatements)
🧹 Nitpick comments (1)
lego-webapp/components/ShareButton/index.tsx (1)
1-2: Remove unused import.The
Share2icon is imported but never used in the component.-import { Button, Flex, Icon } from '@webkom/lego-bricks'; -import { Share2, ShareIcon } from 'lucide-react'; +import { Button, Flex, Icon } from '@webkom/lego-bricks'; +import { ShareIcon } from 'lucide-react';
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
lego-webapp/components/ShareButton/index.tsx(1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
lego-webapp/components/ShareButton/index.tsx
[error] 17-19: This block statement doesn't serve any purpose and can be safely removed.
Standalone block statements without any block-level declarations are redundant in JavaScript and can be removed to simplify the code.
Safe fix: Remove redundant block.
(lint/complexity/noUselessLoneBlockStatements)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (2)
lego-webapp/components/ShareButton/index.tsx (2)
14-15: Enhance Web Share API compatibility checks.Relying solely on
navigator?.canSharemay not be sufficient for all browsers. Some browsers might partially implement the API or have limitations with certain share data types.Consider testing your implementation across different browsers, especially Firefox Desktop which doesn't support the Web Share API at all. The current implementation falls back to clipboard copy when sharing isn't supported, which is good, but it might be worth enhancing the checking mechanism.
For more reliable detection, you could combine checking for both feature existence AND ability to share the specific content:
-if (navigator?.canShare?.({ title, url })) { +if (navigator?.share && navigator.canShare?.({ title, url })) {This ensures both the API is available AND the specific content can be shared via the API.
29-36: LGTM: Clean component rendering.The component rendering is clean and properly structured, using Flex for layout and appropriate spacing.
|
I think it should be together with the other social interactions; comments and reactions. The sidebar is already too cluttered. Imo. we don't need the text either, only the icon alone is sufficient |
Description
Added a ShareButton component and implemented it in events, so people can share the link to different platforms or get it copied to clipboard.
Result
Description:
Added a sharebutton to events under "Food allergies". First after-photo in Safari. Second in Firefox as a link.
Before
After
If you've made visual changes, please check the boxes below and include images showing the changes. Descriptions are appreciated.
Caution
Make sure your images do not contain any real user information.
Testing
Please describe what and how the changes have been tested, and provide instructions to reproduce if necessary.
Resolves ABA-1177