feat: intelligent content auto-detection for SocialShareButton#102
Open
Muneerali199 wants to merge 16 commits intoAOSSIE-Org:mainfrom
Open
feat: intelligent content auto-detection for SocialShareButton#102Muneerali199 wants to merge 16 commits intoAOSSIE-Org:mainfrom
Muneerali199 wants to merge 16 commits intoAOSSIE-Org:mainfrom
Conversation
…Readability.js Implements GitHub Issue AOSSIE-Org#26 — auto-generate share text based on page content. ## What changed ### src/utils/extractContent.js (new) Standalone content-detection utility (zero external dependencies). Detection priority: Title: og:title → twitter:title → article/main h1 → .post-title / .entry-title → document.title Description: og:description → twitter:description → meta[name=description] → body text excerpt (150-200 chars) Results cached per-call for 30 s; cache can be busted via clearContentCache() for SPA navigation. ### src/social-share-button.js - Inlined _ContentDetector IIFE (mirrors extractContent logic, keeps CDN build zero-dep) - Constructor auto-detects title/description when props are omitted - Manual props always override detection (fully backward-compatible) - New autoDetect: false option to opt out entirely - Static SocialShareButton.clearContentCache() for SPA route-change support - Fixed duplicate window.SocialShareButton assignment at end of file - Added ES module export default for Node/bundler compatibility - Added comments to previously-empty catch blocks (fixes lint no-empty rule) ### src/social-share-button-react.jsx - Added autoDetect prop (default: true) - Calls SocialShareButton.clearContentCache() on prop/route change when autoDetect is enabled ### tests/ (new directory) - tests/extractContent.test.js — 16 unit tests covering title/description detection, content root selection, noise stripping, caching, and error resilience - tests/socialShareButton.autodetect.test.js — 12 integration tests covering auto-detection, manual override, autoDetect:false, fallback, updateOptions, and clearContentCache static method All 28 tests pass using Node built-in test runner (node:test) + jsdom. ### README.md Added 'Automatic Content Detection' section documenting detection priority, zero-config usage, manual override, autoDetect:false, and SPA cache clearing. Updated options table with autoDetect entry. ### package.json - Added jsdom devDependency for DOM simulation in tests - Added test script: node --test tests/**/*.test.js - Added src/utils/extractContent.js to files array
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds a zero-dependency content extractor with a 30s per-URL cache, integrates automatic title/description detection into the core SocialShareButton and its React wrapper (enabled by default), exposes cache-busting APIs and a static clearContentCache, adds tests, and documents SPA/cache behavior in README. Changes
Sequence DiagramsequenceDiagram
participant React as SocialShareButton React wrapper
participant App as SocialShareButton
participant Detector as ContentDetector (instance)
participant Extractor as extractContent.js
participant DOM as Document/Page
rect rgba(120, 170, 240, 0.5)
Note over React,App: Initialization (autoDetect: true)
React->>App: construct(options + autoDetect)
App->>Detector: request extraction
end
rect rgba(160, 120, 210, 0.5)
Note over Detector,Extractor: Extraction flow
Detector->>Extractor: extractContent(document)
Extractor->>DOM: query meta/head/article/body
DOM-->>Extractor: nodes/content
Extractor->>Detector: return {title, excerpt, textContent}
end
rect rgba(120, 200, 150, 0.5)
Note over App: Populate options and cache
Detector->>App: provide auto-detected values
App->>App: set _autoTitle/_autoDescription when absent
Detector->>Detector: cache results (30s TTL)
end
rect rgba(200, 160, 100, 0.5)
Note over React,App: updateOptions / cache bust
React->>App: updateOptions({...}, bustCache?)
App->>Detector: clearContentCache() if autoDetect & requested
Detector->>Extractor: invalidate cache
App->>Detector: re-run extraction
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 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: 9
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)
36-61:⚠️ Potential issue | 🟠 MajorReact wrapper relies on
window.SocialShareButtonwhich may be undefined in bundled ES module scenarios.The wrapper accesses
window.SocialShareButton(line 38) to instantiate the component. However, when consumers use ES module imports (import SocialShareButton from 'social-share-button-aossie'), thewindow.SocialShareButtonassignment only happens if the module is loaded as a script tag (seesrc/social-share-button.jslines 974-976).In a bundled React/Next.js app using ES module imports,
window.SocialShareButtonwill beundefined, causing the component to silently fail to initialize.🐛 Proposed fix to import the class directly
import { useEffect, useRef } from 'react'; +import SocialShareButtonCore from './social-share-button.js'; export const SocialShareButton = ({ // ... props }) => { // ... useEffect(() => { if (containerRef.current && !shareButtonRef.current) { - if (typeof window !== 'undefined' && window.SocialShareButton) { - shareButtonRef.current = new window.SocialShareButton({ + if (typeof window !== 'undefined') { + shareButtonRef.current = new SocialShareButtonCore({ container: containerRef.current, // ... options }); } }This ensures the React wrapper works whether the library is loaded via CDN script tags or ES module imports.
🤖 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 36 - 61, The useEffect currently reads window.SocialShareButton which can be undefined when consumers import the package as an ES module; update the React wrapper to import the SocialShareButton class directly and fall back to window.SocialShareButton if the import fails. Concretely, add an import for SocialShareButton from the package entry (use the same exported class used in src/social-share-button.js) and in the useEffect (where containerRef, shareButtonRef, currentUrl, currentTitle, description, etc. are passed), instantiate using the imported SocialShareButton if available, otherwise use window.SocialShareButton as a fallback; ensure the symbol names shareButtonRef and containerRef remain unchanged and keep existing constructor options and fallback behavior.src/social-share-button.js (1)
757-772:⚠️ Potential issue | 🟠 Major
updateOptionsdoes not re-run content detection — cache clearing in React wrapper is ineffective.The React wrapper (lines 76-85 in
social-share-button-react.jsx) callsclearContentCache()beforeupdateOptions()expecting fresh content detection on route changes. However,updateOptionsonly merges options and updates the modal URL — it never re-runs_ContentDetector.extract().This means:
- Auto-detected title/description are captured once at construction time
- Calling
clearContentCache()+updateOptions()won't pick up new page metadata- SPA navigation won't benefit from auto-detection after initial mount
To fix this, either:
- Re-run detection in
updateOptionswhenautoDetectis enabled and title/description are not explicitly provided- Or document that auto-detection only works at construction time
🐛 Proposed fix to re-run detection in updateOptions
updateOptions(options) { + // Re-run content detection if autoDetect is enabled and title/description not provided + if (this.options.autoDetect && typeof document !== 'undefined') { + const needsTitle = !options.title && !this.options.title; + const needsDesc = !options.description && !this.options.description; + if (needsTitle || needsDesc) { + try { + const detected = _ContentDetector.extract(document, true); // bustCache + if (needsTitle) options.title = detected.title || this.options.title; + if (needsDesc) options.description = detected.excerpt || this.options.description; + } catch (_) { + // Never let detection errors break updateOptions + } + } + } + this.options = { ...this.options, ...options };🤖 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 757 - 772, The updateOptions method currently only merges options and updates the modal URL, so it never re-runs content detection; modify updateOptions (method name: updateOptions) to, when this.options.autoDetect is true and incoming options do not explicitly include title or description, call _ContentDetector.extract() (or the existing content-detector API used elsewhere) to re-run detection and merge returned title/description into this.options before updating the modal input and invoking applyCustomColors(); ensure this respects clearContentCache() semantics (i.e., detection runs afresh after cache clear) and update the modal input value after detection if necessary.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@package.json`:
- Line 49: The package.json dependency entry for "jsdom" uses a non-existent
version "^29.0.0"; update the "jsdom" version string in package.json to a valid
release (e.g. "^28.1.0" or "^28.0.0") so npm install succeeds, then reinstall
dependencies (npm install or yarn install) to update the lockfile; locate the
"jsdom" entry in package.json to make the change.
In `@README.md`:
- Around line 496-509: Update the README to clarify that auto-detected values
are captured only at construction time and that calling
SocialShareButton.clearContentCache() followed by
shareButtonRef.current?.updateOptions(...) will not re-run content detection
because updateOptions() does not perform auto-detection; either note this
limitation explicitly next to the SPA example or replace the example with a
pattern that destroys and recreates the instance (e.g., call the instance's
destroy method or unmount and create a new SocialShareButton) so that
constructor-time detection runs again; reference the SocialShareButton class,
clearContentCache(), updateOptions(), and shareButtonRef to make the recommended
change obvious.
- Around line 442-453: The Markdown table is broken because the cell containing
"document.title (strips `| Site Name` suffixes)" has an unescaped pipe
character; fix the table in README.md by either escaping the pipe inside that
cell (replace `|` with `\|`) or by rephrasing that cell to avoid using a raw
pipe (e.g., "strips 'Site Name' suffixes"), and ensure the row for the Title
fallback remains aligned with the three-column table schema so the header and
subsequent rows render correctly.
In `@src/social-share-button-react.jsx`:
- Around line 72-85: clearContentCache() is being called but updateOptions()
never re-runs content detection (the detector only runs in the constructor), so
auto-detected metadata isn't refreshed; change updateOptions in
social-share-button.js (the method named updateOptions) to accept an option/flag
to force re-running the content detector (invoke _ContentDetector.extract() when
autoDetect is true or when the new flag is set) and then, in
social-share-button-react.jsx where you clear the cache (inside the useEffect
with shareButtonRef.current and autoDetect), call updateOptions with that flag
so detection is executed after cache clear.
- Around line 78-85: The auto-detect branch calls
window.SocialShareButton.clearContentCache which can be undefined in
ESM/tree-shaken bundles; replace the direct window check with a robust lookup
that uses globalThis and falls back to importing or requiring the library export
before calling clearContentCache. Specifically, in the autoDetect logic in
social-share-button-react.jsx reference the same exported symbol
(SocialShareButton / clearContentCache) instead of relying solely on window,
attempt to resolve globalThis.SocialShareButton and if missing dynamically
resolve the module export (the package export that provides SocialShareButton)
and invoke clearContentCache only if it exists and is a function.
In `@src/social-share-button.js`:
- Around line 138-143: Replace the empty catch handling around the
_toPlainText(_findContentRoot(doc)) call so it conditionally logs via a private
helper instead of swallowing errors: add a private method on SocialShareButton
named _debugWarn(message, err) that checks this.options?.debug and if true calls
console.warn('[SocialShareButton]', message, err) (with eslint-disable comment
only inside the helper), then change the catch block that currently sets
textContent = excerpt to call this._debugWarn('failed to extract plain text from
document', _) before falling back to excerpt; keep the module-level
ContentDetector IIFE silent or use a module-level debug flag rather than
instance this.options for its errors.
- Around line 920-942: The catch blocks inside _emit (around the CustomEvent
dispatch, the this.options.onAnalytics call, and the loop over
this.options.analyticsPlugins calling plugin.track) should stop being empty
comments and instead, when the debug option is enabled, emit a non-fatal warning
with the error and context; update each catch to check this.options.debug (or
the existing debug flag) and call console.warn with a descriptive message (e.g.,
"social-share: analytics plugin failed", "social-share: onAnalytics handler
threw", "social-share: CustomEvent dispatch failed") including the caught error
and the payload so failures are visible during debugging but still swallowed to
avoid blocking the share flow.
In `@src/utils/extractContent.js`:
- Around line 24-28: The module-level singleton _cache with TTL (CACHE_TTL_MS)
can return stale results across concurrent SSR requests; update the caching to
be keyed by the document identity (e.g., document.url) instead of a single
global _cache: change _cache from a single object to a Map keyed by
document.url, update extractContent to look up and validate the cached entry by
that URL and TTL, and update clearContentCache to accept an optional url
argument (or clear the specific map entry) so cache ops target the proper
document rather than the global singleton.
- Around line 245-251: Add a brief inline comment above the nested try-catch
that uses findContentRoot and extractPlainText to explain the deliberate
fallback to excerpt on error; reference the textContent assignment, the call
sites findContentRoot(doc) and extractPlainText(contentRoot), and mention that
the catch intentionally sets textContent = excerpt when extraction fails to make
the behavior explicit for future readers.
---
Outside diff comments:
In `@src/social-share-button-react.jsx`:
- Around line 36-61: The useEffect currently reads window.SocialShareButton
which can be undefined when consumers import the package as an ES module; update
the React wrapper to import the SocialShareButton class directly and fall back
to window.SocialShareButton if the import fails. Concretely, add an import for
SocialShareButton from the package entry (use the same exported class used in
src/social-share-button.js) and in the useEffect (where containerRef,
shareButtonRef, currentUrl, currentTitle, description, etc. are passed),
instantiate using the imported SocialShareButton if available, otherwise use
window.SocialShareButton as a fallback; ensure the symbol names shareButtonRef
and containerRef remain unchanged and keep existing constructor options and
fallback behavior.
In `@src/social-share-button.js`:
- Around line 757-772: The updateOptions method currently only merges options
and updates the modal URL, so it never re-runs content detection; modify
updateOptions (method name: updateOptions) to, when this.options.autoDetect is
true and incoming options do not explicitly include title or description, call
_ContentDetector.extract() (or the existing content-detector API used elsewhere)
to re-run detection and merge returned title/description into this.options
before updating the modal input and invoking applyCustomColors(); ensure this
respects clearContentCache() semantics (i.e., detection runs afresh after cache
clear) and update the modal input value after detection if necessary.
🪄 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: 3c0353d6-9f97-4a24-b070-da97c8a5411c
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (7)
README.mdpackage.jsonsrc/social-share-button-react.jsxsrc/social-share-button.jssrc/utils/extractContent.jstests/extractContent.test.jstests/socialShareButton.autodetect.test.js
Comment on lines
+72
to
+85
| // Update options when props change (including URL from route changes). | ||
| // Also bust the content-detection cache so the new page's metadata is used. | ||
| useEffect(() => { | ||
| if (shareButtonRef.current) { | ||
| // Invalidate detection cache on every route/prop change so the new | ||
| // page content is picked up when autoDetect is enabled. | ||
| if ( | ||
| autoDetect && | ||
| typeof window !== 'undefined' && | ||
| window.SocialShareButton && | ||
| typeof window.SocialShareButton.clearContentCache === 'function' | ||
| ) { | ||
| window.SocialShareButton.clearContentCache(); | ||
| } |
Contributor
There was a problem hiding this comment.
Cache clearing before updateOptions is ineffective — detection not re-run.
The intent here is good: clear the cache so fresh page metadata can be detected on route changes. However, as noted in the social-share-button.js review, updateOptions() does not re-run _ContentDetector.extract(). Auto-detection only happens once in the constructor.
This means:
clearContentCache()clears the cache ✓updateOptions()merges new options but uses the original auto-detected values ✗- New page metadata is never picked up ✗
This should be addressed in social-share-button.js by having updateOptions optionally re-run detection.
🤖 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 72 - 85, clearContentCache()
is being called but updateOptions() never re-runs content detection (the
detector only runs in the constructor), so auto-detected metadata isn't
refreshed; change updateOptions in social-share-button.js (the method named
updateOptions) to accept an option/flag to force re-running the content detector
(invoke _ContentDetector.extract() when autoDetect is true or when the new flag
is set) and then, in social-share-button-react.jsx where you clear the cache
(inside the useEffect with shareButtonRef.current and autoDetect), call
updateOptions with that flag so detection is executed after cache clear.
- Fix jsdom version to ^28.1.0 (^29.0.0 doesn't exist) - Fix broken README table (unescaped pipe character) - React wrapper: import SocialShareButton directly, fall back to window - updateOptions: re-run content detection when autoDetect enabled - Add debug logging to catch blocks (constructor, updateOptions, _emit) - extractContent: cache keyed by document URL (avoids SSR stale results) - Add inline comments to extractContent.js fallback - Update README SPA example to note auto-detection behavior - clearContentCache accepts optional URL parameter
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (1)
src/social-share-button.js (1)
948-982: 🧹 Nitpick | 🔵 TrivialDebug logging added, but consider consolidating to a helper.
The catch blocks now correctly include debug-conditional logging, addressing the previous review feedback. However, each block has its own
// eslint-disable-next-line no-consolecomment.Per learnings, consider consolidating to a private
_debugWarn(message, err)helper with a single eslint-disable comment to reduce scattered directives.♻️ Proposed helper consolidation
// Add as a private method _debugWarn(message, err) { if (this.options?.debug) { // eslint-disable-next-line no-console console.warn('[SocialShareButton]', message, err); } }Then replace inline logging:
} catch (_err) { - if (this.options.debug) { - // eslint-disable-next-line no-console - console.warn('[SocialShareButton] CustomEvent dispatch failed', _err); - } + this._debugWarn('CustomEvent dispatch failed', _err); }🤖 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 948 - 982, Add a private helper _debugWarn(message, err) on the SocialShareButton class that checks this.options?.debug and, inside that single method, contains the eslint-disable-next-line no-console and a single console.warn call; then replace the three inline debug blocks that warn on CustomEvent dispatch errors, onAnalytics callback errors, and analyticsPlugins plugin errors with calls to this._debugWarn('[CustomEvent] CustomEvent dispatch failed', _err), this._debugWarn('[onAnalytics] callback threw', _err), and this._debugWarn('[AnalyticsPlugin] plugin failed', _err) respectively so the logging behavior is consolidated and the eslint-disable comment exists only once.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@README.md`:
- Around line 512-514: The markdown closing fence "```" flagged by the linter is
part of a nested code block in README.md; fix by adding an explicit language
identifier to the corresponding opening fence (e.g., change the matching opening
"```" to "```json" or "```bash" as appropriate) or replace the nested fences
with an alternate fence style like "~~~" so the static analyzer recognizes the
language; locate the matching pair for the lone "```" and update the opening
fence to include the language specifier or switch the nested block to "~~~".
In `@src/social-share-button.js`:
- Around line 92-102: The linter warns about the arrow function used in the
forEach call inside _toPlainText; replace the inline arrow callback with a
normal named or anonymous function or convert the forEach into a simple for...of
loop to avoid the warning—locate function _toPlainText and change the
.forEach((el) => el.remove()) usage to either .forEach(function (el) {
el.remove(); }) or a for (const el of clone.querySelectorAll(...)) {
el.remove(); } so the same behavior is preserved without the arrow callback.
- Around line 19-21: The singleton _cache in _ContentDetector (and constant
CACHE_TTL_MS) can leak across SSR requests; replace the single _cache variable
with a Map keyed by document.location.href (or similar document URL) like
src/utils/extractContent.js does, then update _ContentDetector.extract() to
read/write per-document entries and update _ContentDetector.clearCache() to
clear only the entry for the given document URL (or clear the whole Map when
appropriate); ensure TTL logic still uses CACHE_TTL_MS when validating map
entries.
- Around line 7-18: Add a clear synchronization comment above the duplicated
_ContentDetector IIFE stating that this logic is intentionally duplicated from
src/utils/extractContent.js for the zero-dependency CDN build and that any
changes to detection priorities, selectors, or excerpt logic must be mirrored in
both places (identifying src/utils/extractContent.js as the source of truth);
include the file name and a brief note about which function/exports in
src/utils/extractContent.js (e.g., extractContent) must be kept in sync so
future maintainers know where to update.
In `@src/utils/extractContent.js`:
- Around line 175-187: The arrow callback in extractPlainText uses concise
syntax `(el) => el.remove()` which can trigger a linter/static-analysis warning
about returning the result of remove(); change the forEach callback to use block
syntax so it does not return a value — e.g., replace `noise.forEach((el) =>
el.remove())` with a block-bodied arrow `noise.forEach((el) => { el.remove();
})` (alternatively use a for...of loop) to silence the warning while keeping
behavior identical.
---
Duplicate comments:
In `@src/social-share-button.js`:
- Around line 948-982: Add a private helper _debugWarn(message, err) on the
SocialShareButton class that checks this.options?.debug and, inside that single
method, contains the eslint-disable-next-line no-console and a single
console.warn call; then replace the three inline debug blocks that warn on
CustomEvent dispatch errors, onAnalytics callback errors, and analyticsPlugins
plugin errors with calls to this._debugWarn('[CustomEvent] CustomEvent dispatch
failed', _err), this._debugWarn('[onAnalytics] callback threw', _err), and
this._debugWarn('[AnalyticsPlugin] plugin failed', _err) respectively so the
logging behavior is consolidated and the eslint-disable comment exists only
once.
🪄 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: d780f00b-488d-47c9-b9fd-d8bd23553d97
📒 Files selected for processing (5)
README.mdpackage.jsonsrc/social-share-button-react.jsxsrc/social-share-button.jssrc/utils/extractContent.js
README.md
Outdated
Comment on lines
+512
to
+514
| ```` | ||
|
|
||
| --- |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Minor: Markdown lint warning about code fence.
The static analysis tool flags line 512 for missing a language specifier. This appears to be a closing fence for a nested documentation example. If the warning persists, consider adding an explicit language identifier or restructuring the nested code blocks.
🧰 Tools
🪛 markdownlint-cli2 (0.21.0)
[warning] 512-512: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@README.md` around lines 512 - 514, The markdown closing fence "```" flagged
by the linter is part of a nested code block in README.md; fix by adding an
explicit language identifier to the corresponding opening fence (e.g., change
the matching opening "```" to "```json" or "```bash" as appropriate) or replace
the nested fences with an alternate fence style like "~~~" so the static
analyzer recognizes the language; locate the matching pair for the lone "```"
and update the opening fence to include the language specifier or switch the
nested block to "~~~".
Contributor
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
…SSR leaks Replace singleton _cache variable with a Map keyed by doc.URL, mirroring the approach in src/utils/extractContent.js. Update clearCache(url) to delete only the keyed entry when a URL is provided, or clear the entire Map otherwise. Addresses CodeRabbit review round 2 (SSR cache leak concern).
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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-react.jsx`:
- Line 9: The component is forcing auto-detection by always passing a title and
an empty description; change the default and conditional logic so the detector
runs when props are omitted: remove the default description = '' and stop
unconditionally using document.title in the constructor; instead only pass title
and description into updateOptions (and into whatever method uses
document.title) when props.title or props.description are explicitly provided.
Update references in the constructor and updateOptions call sites (look for
constructor, updateOptions, props.title, props.description, and any use of
document.title) so omission of props leaves those values undefined and lets the
core detector's priority chain run.
🪄 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: 643ac015-6296-42d8-93e8-88bebec65841
📒 Files selected for processing (1)
src/social-share-button-react.jsx
…props Remove the description='' default and the currentTitle fallback to document.title so that omitting title or description props leaves those values undefined, allowing the core SocialShareButton auto-detection priority chain (og:title → twitter:title → h1 → document.title) to run unimpeded. Addresses CodeRabbit review round 3 comment on social-share-button-react.jsx.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 141-146: The cache key fallback in extract(doc, bustCache = false)
currently uses an empty string; change it to match the source-of-truth in
src/utils/extractContent.js by using 'default' as the fallback (i.e., compute
cacheKey from doc && doc.URL ? doc.URL : 'default') so _cache.get(cacheKey) uses
the same default key across modules and preserves consistent caching behavior
with the extractContent implementation.
🪄 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: 2a7f901b-72af-417b-b95a-d9c1f03c5458
📒 Files selected for processing (3)
README.mdsrc/social-share-button.jssrc/utils/extractContent.js
Change the cacheKey fallback in _ContentDetector.extract() from '' to 'default' to be consistent with the source-of-truth in src/utils/extractContent.js. Addresses CodeRabbit review round 4 comment.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
src/social-share-button.js (1)
786-801:⚠️ Potential issue | 🟠 MajorAuto-detection can go stale after navigation because re-detection is gated on old values.
At Line 789 and Line 790,
needsTitle/needsDescdepend onthis.options.title/description. After initial auto-detection these are non-empty, so route updates skip extraction and keep previous-page metadata.♻️ Proposed fix (check explicit props, not previous resolved values)
updateOptions(options) { // Re-run content detection if autoDetect is enabled and title/description not provided if (this.options.autoDetect && typeof document !== 'undefined') { - const needsTitle = !options.title && !this.options.title; - const needsDesc = !options.description && !this.options.description; + const hasExplicitTitle = Object.prototype.hasOwnProperty.call(options, 'title'); + const hasExplicitDesc = Object.prototype.hasOwnProperty.call(options, 'description'); + const needsTitle = !hasExplicitTitle; + const needsDesc = !hasExplicitDesc; if (needsTitle || needsDesc) { try { const detected = _ContentDetector.extract(document, true); // bustCache if (needsTitle && detected.title) { options.title = detected.title; } if (needsDesc && detected.excerpt) { options.description = detected.excerpt; }🤖 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 786 - 801, updateOptions currently guards re-detection using this.options.title/description so once previous auto-detection populated this.options future updates skip extraction; change the check to base needsTitle and needsDesc solely on the incoming options object (i.e., const needsTitle = !options.title; const needsDesc = !options.description) when this.options.autoDetect is true and document exists, then run _ContentDetector.extract(document, true) and assign detected.title -> options.title and detected.excerpt -> options.description as before so route navigations without explicit props trigger fresh detection.
🤖 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 121-129: Add a short inline comment inside the _excerpt function
explaining the truncation heuristic: note that we first try to cut at the last
sentence boundary within the maxLen window (sentEnd) but only if that boundary
is beyond minLen to avoid too-short snippets, otherwise we fall back to the last
word boundary (wordEnd) and append an ellipsis; reference the variables win,
sentEnd, wordEnd, minLen and maxLen in the comment to make the intent and
edge-case rationale explicit for future maintainers.
- Around line 414-430: The hardcoded share templates (whatsappMessage,
facebookMessage, twitterMessage, telegramMessage, redditTitle, emailBody) must
be moved into i18n resource entries and referenced here by keys; replace literal
English strings and emoji-containing templates with lookups (e.g.,
i18n.t('share.whatsapp', { title, description, hashtags })) or a resource object
and assemble with provided placeholders so translations/locale variants can be
supplied, ensuring you preserve the placeholder names (title, description,
hashtagString) and emoji where needed and add sensible fallback strings if a key
is missing.
- Around line 1039-1041: Remove the redundant CommonJS export that assigns
module.exports = SocialShareButton; because this file is ESM-only and already
uses export default SocialShareButton. Locate the module.exports assignment (the
module.exports symbol and SocialShareButton reference) and delete that block so
the file only exposes the existing export default SocialShareButton; this
prevents misleading CJS compatibility hints.
---
Duplicate comments:
In `@src/social-share-button.js`:
- Around line 786-801: updateOptions currently guards re-detection using
this.options.title/description so once previous auto-detection populated
this.options future updates skip extraction; change the check to base needsTitle
and needsDesc solely on the incoming options object (i.e., const needsTitle =
!options.title; const needsDesc = !options.description) when
this.options.autoDetect is true and document exists, then run
_ContentDetector.extract(document, true) and assign detected.title ->
options.title and detected.excerpt -> options.description as before so route
navigations without explicit props trigger fresh detection.
🪄 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: a58f650e-00b2-4a4f-a826-031ef37a05dc
📒 Files selected for processing (2)
src/social-share-button-react.jsxsrc/social-share-button.js
src/social-share-button.js
Outdated
Comment on lines
414
to
430
| whatsappMessage = `\u{1F680} ${title}${description ? '\n\n' + description : ''}${hashtagString ? '\n\n' + hashtagString : ''}\n\nLive on the site \u{1F440}\nClean UI, smooth flow \u{2014} worth peeking\n\u{1F447}`; | ||
|
|
||
| // Facebook: Title + Description | ||
| facebookMessage = `${title}${description ? "\n\n" + description : ""}${hashtagString ? "\n\n" + hashtagString : ""}`; | ||
| facebookMessage = `${title}${description ? '\n\n' + description : ''}${hashtagString ? '\n\n' + hashtagString : ''}`; | ||
|
|
||
| // Twitter: Title + Description + Hashtags + Via | ||
| twitterMessage = `${title}${description ? "\n\n" + description : ""}${hashtagString ? "\n" + hashtagString : ""}`; | ||
| twitterMessage = `${title}${description ? '\n\n' + description : ''}${hashtagString ? '\n' + hashtagString : ''}`; | ||
|
|
||
| // Telegram: Casual with emoji | ||
| telegramMessage = `\u{1F517} ${title}${description ? "\n\n" + description : ""}${hashtagString ? "\n\n" + hashtagString : ""}\n\nLive + working\nClean stuff, take a look \u{1F447}`; | ||
| telegramMessage = `\u{1F517} ${title}${description ? '\n\n' + description : ''}${hashtagString ? '\n\n' + hashtagString : ''}\n\nLive + working\nClean stuff, take a look \u{1F447}`; | ||
|
|
||
| // Reddit: Title + Description | ||
| redditTitle = `${title}${description ? " - " + description : ""}`; | ||
| redditTitle = `${title}${description ? ' - ' + description : ''}`; | ||
|
|
||
| // Email: Friendly greeting | ||
| emailBody = `Hey \u{1F44B}\n\nSharing a clean project I came across:\n${title}${description ? "\n\n" + description : ""}\n\nLive, simple, and usable \u{2014} take a look \u{1F447}`; | ||
| emailBody = `Hey \u{1F44B}\n\nSharing a clean project I came across:\n${title}${description ? '\n\n' + description : ''}\n\nLive, simple, and usable \u{2014} take a look \u{1F447}`; | ||
|
|
Contributor
There was a problem hiding this comment.
Hardcoded share-message text should be externalized for localization.
These platform templates are user-visible copy but are embedded English literals, which blocks i18n/l10n and regional customization.
As per coding guidelines: "Internationalization: User-visible strings should be externalized to resource files (i18n)."
🤖 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 414 - 430, The hardcoded share
templates (whatsappMessage, facebookMessage, twitterMessage, telegramMessage,
redditTitle, emailBody) must be moved into i18n resource entries and referenced
here by keys; replace literal English strings and emoji-containing templates
with lookups (e.g., i18n.t('share.whatsapp', { title, description, hashtags }))
or a resource object and assemble with provided placeholders so
translations/locale variants can be supplied, ensuring you preserve the
placeholder names (title, description, hashtagString) and emoji where needed and
add sensible fallback strings if a key is missing.
…ment, remove CJS shim - updateOptions: base needsTitle/needsDesc on hasOwnProperty of incoming options so fresh auto-detection runs on route changes even when this.options already has previously-detected values (fixes stale-after-navigation bug) - _excerpt: add inline comment explaining the sentence/word boundary truncation heuristic and the minLen guard to aid future maintainers - Remove dead CommonJS module.exports block (package.json type=module; file is ESM-only; window assignment and export default remain) Addresses CodeRabbit review round 5 actionable comments.
Contributor
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (1)
src/social-share-button.js (1)
416-435:⚠️ Potential issue | 🟠 MajorExternalize platform share-message templates for localization.
User-visible strings are hardcoded English literals, which blocks translation and locale-specific customization.
As per coding guidelines: "Internationalization: User-visible strings should be externalized to resource files (i18n)."
🤖 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 416 - 435, Extract all hardcoded share templates (whatsappMessage, facebookMessage, twitterMessage, telegramMessage, redditTitle, emailBody) into i18n resource keys (e.g., share.whatsapp, share.facebook, share.twitter, share.telegram, share.redditTitle, share.emailBody) and replace the literal string assembly with calls to the localization utility (e.g., i18n.t or t) using placeholders for title, description, and hashtagString so translations can control punctuation, ordering and emojis; ensure each resource string contains placeholders for optional fields and update the code that builds whatsappMessage/twitterMessage/etc. to pass { title, description, hashtagString } to the i18n interpolation function and handle empty/optional placeholders consistently.
🤖 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-react.jsx`:
- Around line 85-97: The code currently calls
ShareButtonConstructor.clearContentCache() with no args which wipes the global
TTL cache; change it to only clear the entry for the current page by calling
clearContentCache with the current URL (e.g.
ShareButtonConstructor.clearContentCache(props.url || window.location.href))
inside the autoDetect branch, ensuring you still guard for
ShareButtonConstructor and typeof clearContentCache === 'function'.
- Around line 1-4: This module uses React client hooks (useEffect, useRef) and
must be marked as a client component for Next.js App Router; add the `'use
client'` directive as the first line of the file (above the imports) so the
component that imports/use SocialShareButtonCore and hooks is treated as a
client module; update src/social-share-button-react.jsx to include `'use
client'` at the top of the file to enable safe import of SocialShareButtonCore
and to allow useEffect/useRef to run client-side.
In `@src/social-share-button.js`:
- Around line 38-107: Several detector helper functions lack descriptive inline
intent notes; add concise comments above _getMeta, _detectTitle,
_detectMetaDesc, _findContentRoot (and _toPlainText if present) that briefly
explain purpose, key heuristics/selector priorities (e.g., prefer og/twitter
meta over landmark selectors, fallback title suffix-stripping regex,
content-root minimum length), and any assumptions or edge-cases they handle so
future reviewers can quickly understand why those selectors/heuristics were
chosen.
- Around line 213-214: The code treats empty strings as "missing" by using
truthy checks; change the checks that set _needsTitle and _needsDesc to test for
undefined/null instead of falsiness (e.g., use options.title === undefined or
options.title == null) so an explicit empty string provided by the caller is
honored; update the same pattern at the other occurrence (the checks at lines
corresponding to the second pair, currently setting variables analogous to
_needsTitle/_needsDesc) to use the same undefined/null check.
- Around line 791-814: updateOptions currently reads this.options.autoDetect
before merging in the incoming options, so toggling autoDetect in the same call
is ignored; fix by computing an effectiveAutoDetect flag from the incoming
options (use options.autoDetect if present, otherwise fall back to
this.options.autoDetect) and use that effectiveAutoDetect when deciding to run
_ContentDetector.extract; keep the rest of the detection logic
(needsTitle/needsDesc, setting options.title/options.description, and catching
errors with this._debugWarn) unchanged.
---
Duplicate comments:
In `@src/social-share-button.js`:
- Around line 416-435: Extract all hardcoded share templates (whatsappMessage,
facebookMessage, twitterMessage, telegramMessage, redditTitle, emailBody) into
i18n resource keys (e.g., share.whatsapp, share.facebook, share.twitter,
share.telegram, share.redditTitle, share.emailBody) and replace the literal
string assembly with calls to the localization utility (e.g., i18n.t or t) using
placeholders for title, description, and hashtagString so translations can
control punctuation, ordering and emojis; ensure each resource string contains
placeholders for optional fields and update the code that builds
whatsappMessage/twitterMessage/etc. to pass { title, description, hashtagString
} to the i18n interpolation function and handle empty/optional placeholders
consistently.
🪄 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: e9b17bfa-1c11-4bb7-949f-be2c6a80ecdc
📒 Files selected for processing (2)
src/social-share-button-react.jsxsrc/social-share-button.js
- React: add 'use client' directive for Next.js App Router compatibility - React: pass currentUrl to clearContentCache() to avoid wiping global cache - JS: add inline JSDoc comments on _getMeta, _detectTitle, _detectMetaDesc, _findContentRoot explaining heuristics, selector priority, and edge-cases - JS constructor: use == null for _needsTitle/_needsDesc so explicit empty strings are honoured and don't trigger auto-detection - JS updateOptions: compute effectiveAutoDetect from incoming options before reading this.options so toggling autoDetect in the same call works immediately
Contributor
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
Contributor
There was a problem hiding this comment.
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-react.jsx (1)
127-147:⚠️ Potential issue | 🟡 MinorConsider using memoization (useMemo/useCallback) in parent components when passing array and callback props.
Arrays and objects are compared by reference in JavaScript. Without memoization, passing literal arrays like
hashtags={['js', 'web']}or non-memoized callbacks will cause this effect to re-run on every parent render, even if the values haven't changed. This triggers unnecessaryupdateOptions()calls on the underlying SocialShareButton instance.Update the component documentation (README) to recommend wrapping dynamic array/callback props with
useMemoanduseCallbackin parent components, or consider using a custom comparison function if the underlying library supports it.🤖 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 127 - 147, The effect dependency list in the SocialShareButton component causes updateOptions() to run whenever parent renders because arrays/callbacks are compared by reference; update the README to advise consumers to wrap array props (e.g., hashtags, platforms, analyticsPlugins) with useMemo and callback props (e.g., onShare, onCopy, onAnalytics) with useCallback in parent components, and optionally add guidance to pass stable componentId/theme/buttonText via useMemo/useCallback as needed; alternatively, inside the component consider a defensive change around updateOptions() (e.g., implement a shallow/deep comparison of previous options before calling SocialShareButton.updateOptions or use refs to store previous array/callback references) so non-mutative re-renders don’t trigger unnecessary updates.
♻️ Duplicate comments (1)
src/social-share-button.js (1)
135-147: 🧹 Nitpick | 🔵 TrivialAdd a brief inline comment for
_toPlainText.This function lacks an explanatory comment. While straightforward, noting why certain elements are removed (e.g., scripts, navigation, ads) aids maintainability.
As per coding guidelines: "Add comments for logical blocks explaining what they do."
📝 Proposed comment
+ // Extract plain text from a DOM subtree, stripping scripts, styles, nav, + // and other non-content elements to get the main body text. function _toPlainText(root) { const clone = root.cloneNode(true);🤖 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 135 - 147, Add a concise inline comment above the _toPlainText function stating its purpose (to produce a cleaned plain-text version of a DOM node) and why specific elements are removed (strip scripts, styles, navigation, ads, ARIA-hidden and modal overlays so they don't pollute shareable text); reference that it clones the root, removes matching selectors via querySelectorAll and then normalizes whitespace from clone.textContent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/social-share-button-react.jsx`:
- Around line 127-147: The effect dependency list in the SocialShareButton
component causes updateOptions() to run whenever parent renders because
arrays/callbacks are compared by reference; update the README to advise
consumers to wrap array props (e.g., hashtags, platforms, analyticsPlugins) with
useMemo and callback props (e.g., onShare, onCopy, onAnalytics) with useCallback
in parent components, and optionally add guidance to pass stable
componentId/theme/buttonText via useMemo/useCallback as needed; alternatively,
inside the component consider a defensive change around updateOptions() (e.g.,
implement a shallow/deep comparison of previous options before calling
SocialShareButton.updateOptions or use refs to store previous array/callback
references) so non-mutative re-renders don’t trigger unnecessary updates.
---
Duplicate comments:
In `@src/social-share-button.js`:
- Around line 135-147: Add a concise inline comment above the _toPlainText
function stating its purpose (to produce a cleaned plain-text version of a DOM
node) and why specific elements are removed (strip scripts, styles, navigation,
ads, ARIA-hidden and modal overlays so they don't pollute shareable text);
reference that it clones the root, removes matching selectors via
querySelectorAll and then normalizes whitespace from clone.textContent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: db8f8d5f-48c1-4cd8-aa54-0d62b3200ea0
📒 Files selected for processing (2)
src/social-share-button-react.jsxsrc/social-share-button.js
…f tip in README - _toPlainText: add inline comment explaining the clone-and-strip approach and why specific element categories (scripts, nav, ads, ARIA-hidden, modal overlays) are removed to avoid polluting shareable text - README: add a Performance tip callout near the React component section advising consumers to wrap array/callback props (hashtags, platforms, analyticsPlugins, onShare, onCopy, onAnalytics) with useMemo/useCallback to prevent unnecessary updateOptions() calls on every parent render Addresses CodeRabbit review round 7 remaining comments.
Contributor
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
Contributor
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
src/social-share-button.js (2)
453-471:⚠️ Potential issue | 🟠 MajorHardcoded share-message templates should be externalized for localization.
These user-facing strings are embedded in English literals, which blocks i18n/l10n customization.
As per coding guidelines: "Internationalization: User-visible strings should be externalized to resource files (i18n)."
🤖 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 453 - 471, The share message templates (whatsappMessage, facebookMessage, twitterMessage, telegramMessage, redditTitle, emailBody) are hardcoded in English; externalize them into i18n resource keys and replace the inline literals with template lookups (e.g., i18n.t('share.whatsapp', { title, description, hashtagString }) or your project's translation helper), constructing messages by injecting title/description/hashtagString into the localized templates; ensure pluralization/fallbacks are handled and keep the emoji and punctuation in the localization strings rather than in code so translators can adapt tone and order.
248-251:⚠️ Potential issue | 🟠 MajorManual empty-string overrides are still not honored in constructor defaults.
Line 270andLine 271use||, sotitle: ''is treated as missing and replaced by detected/document title. This conflicts with the intended “manual props win” behavior.💡 Suggested fix
- title: options.title || _autoTitle || (typeof document !== 'undefined' ? document.title : ''), - description: options.description || _autoDescription || '', + title: options.title ?? (_autoTitle || (typeof document !== 'undefined' ? document.title : '')), + description: options.description ?? (_autoDescription || ''),#!/bin/bash set -euo pipefail # Verify constructor still uses falsy-defaulting for title/description. rg -n "title:\s*options\.title\s*\|\||description:\s*options\.description\s*\|\|" src/social-share-button.js -C2 # Verify auto-detect need checks in constructor. rg -n "const _needsTitle = options\.title == null|const _needsDesc = options\.description == null" src/social-share-button.js -C2Also applies to: 268-271
🤖 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 248 - 251, The constructor currently uses falsy-defaulting (title: options.title || detectedTitle and description: options.description || detectedDesc) which treats an explicit empty string as missing; change these defaults to respect manual empty-string overrides by using a null/undefined check instead (e.g., set title to detectedTitle only when options.title == null, otherwise use options.title; same for description). Update the default assignments in the constructor where title and description are set, keeping the existing _needsTitle and _needsDesc checks (const _needsTitle = options.title == null; const _needsDesc = options.description == null) to drive auto-detection logic. Ensure you adjust the expressions that currently use || to use the null/undefined conditional so manual empty strings are honored.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@README.md`:
- Around line 521-523: Update the README examples/tables so the documented
defaults for title and description reflect the actual behavior when autoDetect
is true (the real default): mention that title and description are auto-detected
values with fallbacks rather than just document.title and '' and update the code
snippets/comments around the options (e.g., url, title, description, buttonText,
autoDetect) so both the table and example snippets consistently state that
autoDetect: true (default) will auto-populate title/description from the page
with fallback values; apply the same correction to the other occurrence
referencing title/description defaults as noted.
In `@src/social-share-button.js`:
- Line 8: Update the maintainer-facing comment inside the ContentDetector IIFE
to correct the typo "INTENTIONALY" to "INTENTIONALLY" so the synchronization
note reads correctly; locate the comment near the ContentDetector IIFE in
src/social-share-button.js and replace the misspelled word only, preserving the
rest of the comment text and formatting.
---
Duplicate comments:
In `@src/social-share-button.js`:
- Around line 453-471: The share message templates (whatsappMessage,
facebookMessage, twitterMessage, telegramMessage, redditTitle, emailBody) are
hardcoded in English; externalize them into i18n resource keys and replace the
inline literals with template lookups (e.g., i18n.t('share.whatsapp', { title,
description, hashtagString }) or your project's translation helper),
constructing messages by injecting title/description/hashtagString into the
localized templates; ensure pluralization/fallbacks are handled and keep the
emoji and punctuation in the localization strings rather than in code so
translators can adapt tone and order.
- Around line 248-251: The constructor currently uses falsy-defaulting (title:
options.title || detectedTitle and description: options.description ||
detectedDesc) which treats an explicit empty string as missing; change these
defaults to respect manual empty-string overrides by using a null/undefined
check instead (e.g., set title to detectedTitle only when options.title == null,
otherwise use options.title; same for description). Update the default
assignments in the constructor where title and description are set, keeping the
existing _needsTitle and _needsDesc checks (const _needsTitle = options.title ==
null; const _needsDesc = options.description == null) to drive auto-detection
logic. Ensure you adjust the expressions that currently use || to use the
null/undefined conditional so manual empty strings are honored.
🪄 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: a59e452d-6ef8-456e-a7e0-423bef373f46
📒 Files selected for processing (2)
README.mdsrc/social-share-button.js
…or i18n readiness Consolidate the six per-platform message strings (whatsappMessage, facebookMessage, twitterMessage, telegramMessage, redditTitle, emailBody) into a single SHARE_TEMPLATES object, making the templates visible as a group and trivially replaceable with an i18n interpolation call. Add a comment directing maintainers to replace the template expressions with their preferred i18n function when locale-specific variants are needed. Addresses persistent CodeRabbit i18n review comment (duplicate).
Contributor
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
…EADME defaults - Use ?? instead of || for options.title and options.description so that an explicit empty string '' is preserved rather than treated as missing - Fix typo: INTENTIONALY -> INTENTIONALLY in sync comment - Update README options table to document auto-detection as the default for title and description when those options are omitted
Contributor
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
Contributor
|
@Muneerali199 could you make an demo for showcase autodetection? also used this format of cdn while preview. |
|
Please resolve the merge conflicts before review. Your PR will only be reviewed by a maintainer after all conflicts have been resolved. 📺 Watch this video to understand why conflicts occur and how to resolve them: |
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
2 participants
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.
✅ Checklist
📝 Description
What changes were made
New content detection module (
src/utils/extractContent.js)og:title,og:description,twitter:title,twitter:description,meta[name=description], semantic HTML (<article>,<main>,[role="main"]), and common CMS selectorsclearContentCache()for SPA navigationUpdated SocialShareButton (
src/social-share-button.js)_ContentDetectorIIFE (keeps CDN build zero-dependency)titleanddescriptionwhen props are not providedautoDetect: falseoption to disable detectionSocialShareButton.clearContentCache()for SPA route changesUpdated React wrapper (
src/social-share-button-react.jsx)autoDetectpropTests (
tests/)Why the change is needed
Currently, users must manually provide
titleanddescriptionfor every share button:This creates friction when:
Related Issue
Fixes #26 — Auto-generate Share Text Based on Content
🧪 Testing
All 28 tests pass:
Run tests with:
npm test📖 Documentation
Updated README.md with new section "Automatic Content Detection" covering:
🔄 Backward Compatibility
✅ Fully backward compatible — all existing usage patterns continue to work exactly as before. Manual
titleanddescriptionprops always override auto-detection.📝 Usage Examples
Zero-config (new)
With manual props (unchanged)
Disable auto-detection
SPA navigation
🚀 Future Improvements (Optional)
<link rel="canonical"><meta name="author">for blog attributionextractContentas named export for consumersSummary by CodeRabbit
New Features
Documentation
Tests
Chores