Add analytics integration to SocialShareButton with customizable even…#65
Merged
kpj2006 merged 6 commits intoAOSSIE-Org:mainfrom Mar 12, 2026
Merged
Add analytics integration to SocialShareButton with customizable even…#65kpj2006 merged 6 commits intoAOSSIE-Org:mainfrom
kpj2006 merged 6 commits intoAOSSIE-Org:mainfrom
Conversation
Contributor
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughAdds a privacy-first analytics subsystem and docs: standardized event schema, three local delivery paths (DOM CustomEvent, onAnalytics callback, analyticsPlugins), built-in adapters (GA4, Mixpanel, Segment, Plausible, PostHog, Custom), React prop wiring, package exposure, and docs. No direct data collection by the library. Changes
Sequence Diagram(s)sequenceDiagram
participant Button as SocialShareButton (Core)
participant Emitter as _emit / Event Path
participant DOM as DOM CustomEvent
participant Callback as onAnalytics Callback
participant Registry as analyticsPlugins Registry
participant Adapter as Adapter (GA4/Mixpanel/...)
Button->>Emitter: trigger (share / copy / modal / error)
Emitter->>DOM: dispatch CustomEvent with payload
Emitter->>Callback: invoke onAnalytics(payload)
Emitter->>Registry: for each plugin -> plugin.track(payload)
Registry->>Adapter: adapter maps payload -> provider SDK (if present)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/social-share-button.js (2)
364-385:⚠️ Potential issue | 🟠 MajorCheck the fallback copy result before reporting success.
document.execCommand("copy")can returnfalsewithout throwing. Line 367 currently still shows “Copied!” and emitssocial_share_copyeven when nothing reached the clipboard.📋 Suggested fix
- document.execCommand("copy"); + const copied = document.execCommand("copy"); + if (!copied) { + throw new Error("document.execCommand('copy') returned false"); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/social-share-button.js` around lines 364 - 385, The code treats document.execCommand("copy") as always successful; change it to capture its boolean return value (e.g., const success = document.execCommand("copy")) and only perform the success branch (setting copyBtn.textContent to "Copied!", copyBtn.classList.add("copied"), this._emit("social_share_copy","copy"), and calling this.options.onCopy) when success === true; if success is false, treat it like an error path: log or emit this._emit("social_share_error", "error", { errorMessage: "Copy failed: execCommand returned false" }) and avoid changing the button to "Copied!". Ensure the existing catch still handles thrown exceptions from selection/execCommand.
304-320:⚠️ Potential issue | 🟠 MajorOnly emit
social_share_successafter the popup actually opens.
window.open()returnsnullwhen the browser blocks the popup, but this path still emits success and callsonShare. That will overcount successful shares and suppress the exact error this analytics feature is trying to surface.🚪 Suggested success/error split
if (shareUrl) { this._emit("social_share_click", "share", { platform }); if (platform === "email") { window.location.href = shareUrl; + this._emit("social_share_success", "share", { platform }); + if (this.options.onShare) { + this.options.onShare(platform, this.options.url); + } } else { - window.open( + const shareWindow = window.open( shareUrl, "_blank", "noopener,noreferrer,width=600,height=600", ); + if (!shareWindow) { + this._emit("social_share_error", "error", { + platform, + errorMessage: `Share popup was blocked for platform: ${platform}`, + }); + return; + } + this._emit("social_share_success", "share", { platform }); + if (this.options.onShare) { + this.options.onShare(platform, this.options.url); + } } - - this._emit("social_share_success", "share", { platform }); - - if (this.options.onShare) { - this.options.onShare(platform, this.options.url); - }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/social-share-button.js` around lines 304 - 320, The code currently emits social_share_success and calls this.options.onShare regardless of whether window.open actually opened a popup; change the logic so that for platform === "email" you keep the current behavior and emit social_share_success and call this.options.onShare(platform, this.options.url) after setting window.location.href, but for non-email platforms capture the return value of window.open(shareUrl, "_blank", "noopener,noreferrer,width=600,height=600") into a variable (e.g., popup), check if popup is non-null and not closed: if popup is truthy emit social_share_success and call this.options.onShare(...), otherwise emit a social_share_error (or similar event name) and do not call this.options.onShare; use the existing this._emit method and the same event/context keys (platform) to keep telemetry consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/copilot/integrate-analytics.prompt.md:
- Around line 266-276: The provided example only swaps analyticsPlugins and
doesn't actually gate emission; update the example and docs to initialize
SocialShareButton with analytics disabled and a consent-aware callback that
prevents emitEvent until consent is granted, e.g. create the instance with
analytics: false (or an analytics callback that returns false/no-op) and then
enable analytics later via the public method that toggles emission (referencing
SocialShareButton, analyticsPlugins, emitEvent and the instance method
setAnalyticsEnabled or enableAnalytics) so events are suppressed until consent
is explicitly turned on; ensure the doc example shows initializing with
analytics disabled and then calling the enable method after consent.
- Around line 300-310: The provider naming table is out of sync with what the
adapters actually emit: the adapters in src/social-share-analytics.js forward
payload.eventName unchanged (see uses of payload.eventName where Mixpanel and
Segment are invoked), so update the Mixpanel/Segment column entries to use the
exact GA4/custom event names (snake_case like social_share_click,
social_share_success, social_share_copy, social_share_popup_open) instead of
title-cased variants so the table matches the adapters' output.
In `@docs/client-guide.md`:
- Around line 24-28: Replace the first fenced block (the three-step prose list)
with a plain Markdown list (remove the surrounding triple-backticks) so it's
treated as prose, and update the install snippet that shows npm install to use a
bash fence (change ``` to ```bash) — apply the same bash-fence change to the
similar install snippet at the other occurrence noted (the block around lines
70-72) so markdownlint stops flagging the blocks.
- Around line 32-35: The guide currently points consumers to a repo-only Copilot
prompt path `.github/copilot/integrate-social-share-button.prompt.md` under the
"Using GitHub Copilot?" section which won't exist in installed packages; update
the instruction to reference a published or consumable location instead (e.g.,
add a public URL to the prompt, embed the prompt text into the package README or
docs, or provide a link to the raw file on the repository host), and ensure the
same fix is applied for the other occurrence referenced at lines 51-53; look for
the "Using GitHub Copilot?" heading and the `.github/copilot/*.prompt.md`
references to replace with the new public link or embedded content.
In `@src/social-share-button.js`:
- Around line 579-589: The three empty catch blocks around CustomEvent
creation/dispatch (in the block that constructs domEvent and calls
this._getContainer().dispatchEvent) are currently swallowing errors and
violating the no-empty rule; replace each empty catch with a minimal,
intentional handler: either log a debug-only warning (e.g., using console.debug
or a provided logger) that includes the error and context ("social-share
analytics event failed" and the payload), or add a concise explanatory comment
inside the catch explaining why the error is intentionally ignored—ensure the
handler is lint-clean (no empty blocks) and reference the CustomEvent creation,
this._getContainer(), and dispatchEvent locations when updating the code.
---
Outside diff comments:
In `@src/social-share-button.js`:
- Around line 364-385: The code treats document.execCommand("copy") as always
successful; change it to capture its boolean return value (e.g., const success =
document.execCommand("copy")) and only perform the success branch (setting
copyBtn.textContent to "Copied!", copyBtn.classList.add("copied"),
this._emit("social_share_copy","copy"), and calling this.options.onCopy) when
success === true; if success is false, treat it like an error path: log or emit
this._emit("social_share_error", "error", { errorMessage: "Copy failed:
execCommand returned false" }) and avoid changing the button to "Copied!".
Ensure the existing catch still handles thrown exceptions from
selection/execCommand.
- Around line 304-320: The code currently emits social_share_success and calls
this.options.onShare regardless of whether window.open actually opened a popup;
change the logic so that for platform === "email" you keep the current behavior
and emit social_share_success and call this.options.onShare(platform,
this.options.url) after setting window.location.href, but for non-email
platforms capture the return value of window.open(shareUrl, "_blank",
"noopener,noreferrer,width=600,height=600") into a variable (e.g., popup), check
if popup is non-null and not closed: if popup is truthy emit
social_share_success and call this.options.onShare(...), otherwise emit a
social_share_error (or similar event name) and do not call this.options.onShare;
use the existing this._emit method and the same event/context keys (platform) to
keep telemetry consistent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 83e8d384-0982-4522-8c8d-6db8465ca6a7
📒 Files selected for processing (6)
.github/copilot/integrate-analytics.prompt.mddocs/client-guide.mdpackage.jsonsrc/social-share-analytics.jssrc/social-share-button-react.jsxsrc/social-share-button.js
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/social-share-button-react.jsx (1)
1-3: 🧹 Nitpick | 🔵 TrivialMissing
"use client"directive for Next.js compatibility.Per coding guidelines, files matching
**/*.{ts,tsx,js,jsx}should include the"use client"directive. This component uses React hooks (useEffect,useRef) which are client-side only, so adding the directive ensures proper behavior in Next.js App Router environments.🔧 Proposed fix
+"use client"; + import { useEffect, useRef } from "react"; export const SocialShareButton = ({🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/social-share-button-react.jsx` around lines 1 - 3, Add the Next.js client directive at the top of the SocialShareButton component file: insert the single line "use client" before any imports so React hooks (useEffect, useRef) run on the client; update the file containing the SocialShareButton export to start with the directive to ensure Next.js App Router treats this component as a client component.src/social-share-button.js (1)
358-362:⚠️ Potential issue | 🟡 MinorMissing
social_share_popup_openevent inopenModal().The
closeModal()method correctly emitssocial_share_popup_close, butopenModal()doesn't emit the correspondingsocial_share_popup_openevent. According to the analytics schema insocial-share-analytics.js, both events are expected.🔧 Proposed fix in openModal()
openModal() { // Safety check: prevent errors if modal was destroyed if (!this.modal) return; this.isModalOpen = true; this.modal.style.display = "flex"; + this._emit("social_share_popup_open", "popup_open"); // Shared body overflow management...🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/social-share-button.js` around lines 358 - 362, The openModal() method is missing the analytics event emission counterpart to closeModal(); update openModal() to emit the "social_share_popup_open" event via this._emit with the appropriate action (e.g., "popup_open") so it matches the schema in social-share-analytics.js and mirrors closeModal() behavior; locate the openModal() function and add a call like this._emit("social_share_popup_open", "popup_open") at the point where the modal is activated.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/social-share-button.js`:
- Around line 746-749: Add an ESLint suppression comment immediately above the
guarded console statement to satisfy the lint rule: inside the block that checks
this.options.debug (where console.log("[SocialShareButton Analytics]", payload)
is called), insert a single-line comment // eslint-disable-next-line no-console
directly above the console.log so the debug logging remains but the no-console
rule is suppressed for that specific line.
---
Outside diff comments:
In `@src/social-share-button-react.jsx`:
- Around line 1-3: Add the Next.js client directive at the top of the
SocialShareButton component file: insert the single line "use client" before any
imports so React hooks (useEffect, useRef) run on the client; update the file
containing the SocialShareButton export to start with the directive to ensure
Next.js App Router treats this component as a client component.
In `@src/social-share-button.js`:
- Around line 358-362: The openModal() method is missing the analytics event
emission counterpart to closeModal(); update openModal() to emit the
"social_share_popup_open" event via this._emit with the appropriate action
(e.g., "popup_open") so it matches the schema in social-share-analytics.js and
mirrors closeModal() behavior; locate the openModal() function and add a call
like this._emit("social_share_popup_open", "popup_open") at the point where the
modal is activated.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 501f6517-1717-453b-be2a-ae1e7f9057d4
📒 Files selected for processing (3)
package.jsonsrc/social-share-button-react.jsxsrc/social-share-button.js
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
docs/client-guide.md (1)
73-75:⚠️ Potential issue | 🟡 MinorAdd language identifier to the code fence.
The npm install snippet should be tagged as
bashfor proper syntax highlighting and to satisfy linting rules.📝 Suggested fix
-``` +```bash npm install social-share-button-aossie</details> <details> <summary>🧰 Tools</summary> 🪛 markdownlint-cli2 (0.21.0) [warning] 73-73: Fenced code blocks should have a language specified (MD040, fenced-code-language) </details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In
@docs/client-guide.mdaround lines 73 - 75, The fenced code block containing
the npm install command lacks a language tag; update the snippet around thenpm install social-share-button-aossieline to include thebashlanguage
identifier on the opening fence (i.e., change the opening "" to "bash") so
the code block is properly highlighted and satisfies the MD040 lint rule.</details> </blockquote></details> </blockquote></details> <details> <summary>🤖 Prompt for all review comments with AI agents</summary>Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/copilot/integrate-analytics.prompt.md:
- Around line 252-261: Update the docs and implementation to clarify that debug:
true not only logs emitted events but also logs warnings for analytics delivery
failures; specifically, in the SocialShareButton event delivery paths (e.g.,
CustomEvent dispatch, plugin callback invocation, and any plugin/error catch
blocks) add debug-conditional console.warn() calls inside each catch handler to
surface delivery or callback exceptions with a prefixed message like
"[SocialShareButton Analytics] Warning:" so developers see both event payloads
and warning details when debug is enabled.- Around line 44-51: The openModal() method is missing the analytics emission
corresponding to the documentedsocial_share_popup_openevent; add a call to
_emit("social_share_popup_open", "popup_open") inside the openModal() function
(mirroring closeModal() which emitssocial_share_popup_close) so opening the
share modal fires the correct analytics event and matches the documented
contract.In
@docs/client-guide.md:
- Line 30: The sentence "That's it. No accounts. No API keys. No configuration
files." repeats the same sentence-start and should be varied; update that line
in docs/client-guide.md by rephrasing one or more clauses to avoid three
successive sentences beginning with "No" (for example combine into "That's it —
no accounts, API keys, or configuration files." or change a clause to "Nothing
to set up: no accounts, API keys, or config files."), keeping the same meaning
and concise marketing tone.
Duplicate comments:
In@docs/client-guide.md:
- Around line 73-75: The fenced code block containing the npm install command
lacks a language tag; update the snippet around thenpm install social-share-button-aossieline to include thebashlanguage identifier on
the opening fence (i.e., change the opening "" to "bash") so the code
block is properly highlighted and satisfies the MD040 lint rule.</details> <details> <summary>🪄 Autofix (Beta)</summary> Fix all unresolved CodeRabbit comments on this PR: - [ ] <!-- {"checkboxId": "4b0d0e0a-96d7-4f10-b296-3a18ea78f0b9"} --> Push a commit to this branch (recommended) - [ ] <!-- {"checkboxId": "ff5b1114-7d8c-49e6-8ac1-43f82af23a33"} --> Create a new PR with the fixes </details> --- <details> <summary>ℹ️ Review info</summary> <details> <summary>⚙️ Run configuration</summary> **Configuration used**: Path: .coderabbit.yaml **Review profile**: ASSERTIVE **Plan**: Pro **Run ID**: `22639ce1-1e8b-4970-986b-6a360c309509` </details> <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between e0c8f18afdf06d568a7822b8fa80c0029d11d855 and f23ae46c8f2c3c7ccb6b651e233613bb43115f1f. </details> <details> <summary>📒 Files selected for processing (2)</summary> * `.github/copilot/integrate-analytics.prompt.md` * `docs/client-guide.md` </details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/social-share-button.js (1)
403-425:⚠️ Potential issue | 🟡 MinorThe
social_share_successevent is emitted optimistically.
window.opencan returnnullif blocked by a popup blocker, but the success event is still emitted. Consider verifying the popup actually opened, or rename tosocial_share_attemptfor clearer semantics.🔧 Proposed fix to verify popup opened
if (platform === "email") { window.location.href = shareUrl; + this._emit("social_share_success", "share", { platform }); } else { - window.open( + const popup = window.open( shareUrl, "_blank", "noopener,noreferrer,width=600,height=600", ); + if (popup) { + this._emit("social_share_success", "share", { platform }); + } else { + this._emit("social_share_error", "error", { + platform, + errorMessage: "Popup blocked or failed to open", + }); + } } - - this._emit("social_share_success", "share", { platform }); if (this.options.onShare) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/social-share-button.js` around lines 403 - 425, The success event is emitted without verifying the popup opened; change the logic in the block where you call window.open (and the email branch) to check the return value and emit appropriately: call this._emit("social_share_click", "share", { platform }) as before, then for platform === "email" set window.location.href and emit "social_share_success" (or "social_share_attempt" if you prefer renaming), but for non-email use const popup = window.open(shareUrl, "_blank", "noopener,noreferrer,width=600,height=600") and only emit this._emit("social_share_success", "share", { platform }) and call this.options.onShare(platform, this.options.url) if popup is non-null; if popup is null emit this._emit("social_share_error", "error", { platform, errorMessage: "Popup blocked" }) instead (or rename to social_share_attempt consistently if you choose that route).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/social-share-button.js`:
- Around line 700-705: The _getContainer method should be made SSR-safe by
adding an explicit document guard: if this.options.container is falsy return
null as now, but before calling document.querySelector ensure typeof document
!== "undefined" and return null if document is unavailable; otherwise keep the
existing behavior (use document.querySelector when this.options.container is a
string, or return the provided DOM element). Update _getContainer accordingly so
it is self-contained (in addition to the existing typeof window check in _emit).
---
Outside diff comments:
In `@src/social-share-button.js`:
- Around line 403-425: The success event is emitted without verifying the popup
opened; change the logic in the block where you call window.open (and the email
branch) to check the return value and emit appropriately: call
this._emit("social_share_click", "share", { platform }) as before, then for
platform === "email" set window.location.href and emit "social_share_success"
(or "social_share_attempt" if you prefer renaming), but for non-email use const
popup = window.open(shareUrl, "_blank",
"noopener,noreferrer,width=600,height=600") and only emit
this._emit("social_share_success", "share", { platform }) and call
this.options.onShare(platform, this.options.url) if popup is non-null; if popup
is null emit this._emit("social_share_error", "error", { platform, errorMessage:
"Popup blocked" }) instead (or rename to social_share_attempt consistently if
you choose that route).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: eb8cea8a-dd6e-4f74-975a-0113e7cb98a4
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (1)
src/social-share-button.js
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
3 tasks
This was referenced Mar 12, 2026
Closed
4 tasks
This was referenced Mar 17, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
…t tracking
Addressed Issues:
Fixes #(TODO:issue number)
Screenshots/Recordings:
Additional Notes:
Checklist
AI Usage Disclosure
Check one of the checkboxes below:
I have used the following AI models and tools: TODO
We encourage contributors to use AI tools responsibly when creating Pull Requests. While AI can be a valuable aid, it is essential to ensure that your contributions meet the task requirements, build successfully, include relevant tests, and pass all linters. Submissions that do not meet these standards may be closed without warning to maintain the quality and integrity of the project. Please take the time to understand the changes you are proposing and their impact. AI slop is strongly discouraged and may lead to banning and blocking. Do not spam our repos with AI slop.
Summary by CodeRabbit
New Features
Documentation
Chore