Added Qwiks/ QwikCity Integration — new wrapper component and demo pa…#120
Added Qwiks/ QwikCity Integration — new wrapper component and demo pa…#120HackerAaditya wants to merge 2 commits intoAOSSIE-Org:mainfrom
Conversation
…ge section.close#57
WalkthroughAdds Qwik/QwikCity integration: new Qwik wrapper component that initializes the global SocialShareButton client-side via Changes
Sequence Diagram(s)sequenceDiagram
participant SSR as Server-rendered page
participant Browser as Browser (Client)
participant QwikComp as SocialShareButton (Qwik component)
participant Global as window.SocialShareButton (Global constructor)
SSR->>Browser: Serve HTML with demo section and Qwik script
Browser->>QwikComp: hydrate / mount (client)
QwikComp->>QwikComp: useVisibleTask$ runs (client-only)
QwikComp->>Global: check existence
alt Global exists
QwikComp->>Global: new SocialShareButton({container, options})
Global-->>QwikComp: instance
QwikComp-->>Browser: renders button UI into container
Note right of QwikComp: cleanup registers destroy()
else Global missing
QwikComp-->>Browser: no-op (no instantiation)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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)
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 Tip You can generate walkthrough in a markdown collapsible section to save space.Enable the |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 (1)
package.json (1)
6-13:⚠️ Potential issue | 🟠 MajorNew Qwik wrapper file missing from npm
filesarray.The new
src/social-share-button-qwik.tsxfile is not included in thefilesarray, so it won't be published to npm. Users installing via npm won't be able to use the Qwik wrapper.Based on learnings: new wrapper files added in a PR are not part of any CDN release until the next version tag is cut, and acquisition instructions should be provided for the new file itself.
🔧 Proposed fix
"files": [ "src/social-share-button.js", "src/social-share-button.css", "src/social-share-button-react.jsx", + "src/social-share-button-qwik.tsx", "src/social-share-analytics.js", "README.md", "LICENSE" ],🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@package.json` around lines 6 - 13, The package.json "files" array is missing the new Qwik wrapper file; add "src/social-share-button-qwik.tsx" to the "files" array so the new Qwik wrapper (src/social-share-button-qwik.tsx) is included in npm publishes, and update any release/acquisition docs or README references to note the new file name so consumers can install/use the Qwik wrapper after publishing.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@index.html`:
- Around line 447-449: The empty <div class="demo-grid"> indicates an unfinished
demo area; either remove this container or populate it with a clear placeholder
message indicating the Qwik demo requires a Qwik app context (e.g., a short
explanatory paragraph or a link to the live demo) so users aren't shown an empty
block; update the HTML where the demo-grid element is defined (class
"demo-grid") to include the chosen placeholder or remove the element entirely.
- Line 452: The <button class="copy-btn" aria-label="Copy code"> element
defaults to type="submit", which can trigger form submission; update the element
referenced by class "copy-btn" (the Copy button) to include type="button" so it
does not submit forms accidentally, i.e., add the type="button" attribute to
that button element.
- Around line 470-487: The inline Qwik hook usage (useVisibleTask$, component$,
useSignal) is invalid at module scope and will throw at runtime; remove the
entire Qwik module script or replace it with a vanilla DOM-ready initializer
that uses window.SocialShareButton instead; if you choose the vanilla path,
ensure an actual container element with id "demo-qwik" is added to the demo grid
and instantiate SocialShareButton on DOMContentLoaded using the same options
(container '#demo-qwik', url, title, description, buttonText, theme) so the demo
works without a Qwik runtime.
In `@package.json`:
- Around line 51-53: The package currently lists "@builder.io/qwik" under
"dependencies" which forces consumers to install Qwik; move "@builder.io/qwik"
from the "dependencies" object into "peerDependencies" (and optionally add it to
"devDependencies" for local tests) so Qwik is optional for consumers while
preserving the project's zero-deps design; update the package.json entries
accordingly and run npm/yarn install to validate the change.
- Line 45: TypeScript is defaulting to React's JSX namespace due to the presence
of "@types/react", causing type conflicts in the Qwik component (e.g.,
social-share-button-qwik.tsx); add a tsconfig.json at the repository root with
compilerOptions that set "jsx" to "react-jsx" and "jsxImportSource" to
"@builder.io/qwik" so the Qwik JSX namespace is used for .tsx files, then commit
that tsconfig.json to the repo.
In `@README.md`:
- Line 76: Fix the missing space in the framework list string "⚛️ Framework
support: React, Next.js, Vue, Angular,Qwik or plain HTML" by adding a space
after the comma so it reads "Angular, Qwik"; update the README line that
contains that exact string to include the space.
In `@src/social-share-button-qwik.tsx`:
- Around line 40-48: The default platforms array in the Qwik wrapper is missing
'pinterest', causing inconsistency with the core SocialShareButton; update the
default value for props.platforms in src/social-share-button-qwik.tsx (the
object where platforms: props.platforms || [...]) to include 'pinterest'
alongside 'whatsapp','facebook','twitter','linkedin','telegram','reddit' so the
Qwik SocialShareButton mirrors the core SocialShareButton defaults.
- Around line 16-17: Props interface defines onShare and onCopy but those
callbacks aren't forwarded into the options for new window.SocialShareButton;
update the SocialShareButton instantiation to include onShare: props.onShare and
onCopy: props.onCopy (or the local variables holding those QRLs) in the options
object passed to the constructor so the SocialShareButton instance receives and
can invoke them.
- Around line 27-61: Add inline JSDoc-style comments above the SocialShareButton
component and inside the useVisibleTask$ callback: describe what the
SocialShareButton component does, list its Props (url, title, description,
hashtags, via, platforms, theme, buttonText, customClass, buttonStyle,
modalPosition) and its return value (a div container ref), and explain the
useVisibleTask$ logic (checks for window.SocialShareButton, initializes the
external widget via new window.SocialShareButton with the mapped props, and
registers cleanup by calling shareButton.destroy()). Also comment the container
signal (container: useSignal<HTMLDivElement | null>) to note why null is used
and its role as the widget mount point.
---
Outside diff comments:
In `@package.json`:
- Around line 6-13: The package.json "files" array is missing the new Qwik
wrapper file; add "src/social-share-button-qwik.tsx" to the "files" array so the
new Qwik wrapper (src/social-share-button-qwik.tsx) is included in npm
publishes, and update any release/acquisition docs or README references to note
the new file name so consumers can install/use the Qwik wrapper after
publishing.
🪄 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: 348d691d-3f9e-48ac-80df-b8a6c5ea1277
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (5)
README.mdeslint.config.jsindex.htmlpackage.jsonsrc/social-share-button-qwik.tsx
| <div class="demo-grid"> | ||
|
|
||
| </div> |
There was a problem hiding this comment.
Empty demo-grid suggests incomplete implementation.
The demo-grid div is empty. If a live Qwik demo isn't feasible on this static page, consider either removing the empty container or adding a placeholder explaining that the Qwik demo requires a Qwik application context.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@index.html` around lines 447 - 449, The empty <div class="demo-grid">
indicates an unfinished demo area; either remove this container or populate it
with a clear placeholder message indicating the Qwik demo requires a Qwik app
context (e.g., a short explanatory paragraph or a link to the live demo) so
users aren't shown an empty block; update the HTML where the demo-grid element
is defined (class "demo-grid") to include the chosen placeholder or remove the
element entirely.
| </div> | ||
|
|
||
| <div class="code-block"> | ||
| <button class="copy-btn" aria-label="Copy code">Copy</button> |
There was a problem hiding this comment.
Add type="button" to prevent accidental form submission.
Per HTMLHint: the type attribute should be present on <button> elements. Without it, buttons default to type="submit" which can cause unexpected form submissions.
🔧 Proposed fix
- <button class="copy-btn" aria-label="Copy code">Copy</button>
+ <button type="button" class="copy-btn" aria-label="Copy code">Copy</button>📝 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.
| <button class="copy-btn" aria-label="Copy code">Copy</button> | |
| <button type="button" class="copy-btn" aria-label="Copy code">Copy</button> |
🧰 Tools
🪛 HTMLHint (1.9.2)
[warning] 452-452: The type attribute must be present on elements.
(button-type-require)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@index.html` at line 452, The <button class="copy-btn" aria-label="Copy code">
element defaults to type="submit", which can trigger form submission; update the
element referenced by class "copy-btn" (the Copy button) to include
type="button" so it does not submit forms accidentally, i.e., add the
type="button" attribute to that button element.
| <!-- Load Qwik for the demo --> | ||
| <script type="module"> | ||
| import { component$, useVisibleTask$, useSignal } from 'https://cdn.jsdelivr.net/npm/@builder.io/qwik@1.0.0/dist/qwik.mjs'; | ||
| // Demo | ||
| useVisibleTask$(() => { | ||
| const container = document.querySelector('#demo-qwik'); | ||
| if (container && window.SocialShareButton) { | ||
| new window.SocialShareButton({ | ||
| container, | ||
| url: 'https://github.com/AOSSIE-Org/SocialShareButton', | ||
| title: 'SocialShareButton - Qwik Demo', | ||
| description: 'Demo of SocialShareButton in Qwik', | ||
| buttonText: 'Share', | ||
| theme: 'dark' | ||
| }); | ||
| } | ||
| }); | ||
| </script> |
There was a problem hiding this comment.
Broken Qwik demo script: useVisibleTask$ cannot be used outside a component.
The inline script attempts to call useVisibleTask$ at module scope, but Qwik hooks must be called inside a component$ function. This code will throw a runtime error. Additionally, the #demo-qwik container referenced doesn't exist in the HTML.
Since this is a demo page (not a real Qwik app), the Qwik framework hooks won't work without a full Qwik runtime setup. Consider either:
- Removing this script entirely since the code-block documentation above already shows proper usage.
- Adding a simple vanilla JS demo similar to other sections using
DOMContentLoaded.
🔧 Proposed fix: Remove broken script or use vanilla JS
Option 1: Remove the broken script entirely
-<!-- Load Qwik for the demo -->
-<script type="module">
- import { component$, useVisibleTask$, useSignal } from 'https://cdn.jsdelivr.net/npm/@builder.io/qwik@1.0.0/dist/qwik.mjs';
- // Demo
- useVisibleTask$(() => {
- const container = document.querySelector('#demo-qwik');
- if (container && window.SocialShareButton) {
- new window.SocialShareButton({
- container,
- url: 'https://github.com/AOSSIE-Org/SocialShareButton',
- title: 'SocialShareButton - Qwik Demo',
- description: 'Demo of SocialShareButton in Qwik',
- buttonText: 'Share',
- theme: 'dark'
- });
- }
- });
-</script>Option 2: If you want a live demo, add container and use vanilla JS
Add this to the demo-grid:
<div class="demo-item">
<h3>Qwik Demo</h3>
<p>SSR-safe integration</p>
<div id="demo-qwik"></div>
</div>Then in the existing DOMContentLoaded script block, add:
// Qwik Demo
new SocialShareButton({
container: '#demo-qwik',
url: demoUrl,
title: demoTitle,
description: demoDescription,
buttonText: 'Share',
theme: 'dark'
});📝 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.
| <!-- Load Qwik for the demo --> | |
| <script type="module"> | |
| import { component$, useVisibleTask$, useSignal } from 'https://cdn.jsdelivr.net/npm/@builder.io/qwik@1.0.0/dist/qwik.mjs'; | |
| // Demo | |
| useVisibleTask$(() => { | |
| const container = document.querySelector('#demo-qwik'); | |
| if (container && window.SocialShareButton) { | |
| new window.SocialShareButton({ | |
| container, | |
| url: 'https://github.com/AOSSIE-Org/SocialShareButton', | |
| title: 'SocialShareButton - Qwik Demo', | |
| description: 'Demo of SocialShareButton in Qwik', | |
| buttonText: 'Share', | |
| theme: 'dark' | |
| }); | |
| } | |
| }); | |
| </script> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@index.html` around lines 470 - 487, The inline Qwik hook usage
(useVisibleTask$, component$, useSignal) is invalid at module scope and will
throw at runtime; remove the entire Qwik module script or replace it with a
vanilla DOM-ready initializer that uses window.SocialShareButton instead; if you
choose the vanilla path, ensure an actual container element with id "demo-qwik"
is added to the demo grid and instantiate SocialShareButton on DOMContentLoaded
using the same options (container '#demo-qwik', url, title, description,
buttonText, theme) so the demo works without a Qwik runtime.
| "homepage": "https://github.com/AOSSIE-Org/SocialShareButton#readme", | ||
| "devDependencies": { | ||
| "@eslint/js": "^9.39.4", | ||
| "@types/react": "^19.2.14", |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
How to configure tsconfig.json for Qwik JSX with @builder.io/qwik``
💡 Result:
To configure tsconfig.json for Qwik JSX with @builder.io/qwik, use the standard TypeScript configuration from Qwik starter projects. The key settings for Qwik JSX support are "jsx": "react-jsx" and "jsxImportSource": "@builder.io/qwik". Example tsconfig.json: { "compilerOptions": { "target": "ES2022", "lib": ["es2022", "DOM", "DOM.Iterable"], "module": "ESNext", "moduleResolution": "bundler", "allowImportingTsExtensions": true, "verbatimModuleSyntax": true, "noEmit": true, "isolatedModules": true, "jsx": "react-jsx", "jsxImportSource": "@builder.io/qwik", "strict": true, "noUnusedLocals": true, "noUnusedParameters": true, "noFallthroughCasesInSwitch": true, "skipLibCheck": true, "baseUrl": ".", "paths": { "~/": ["./src/"] } }, "include": ["src//.ts", "src//.tsx", "src//.js", "src//.jsx"], "exclude": ["node_modules"] } This configuration enables Qwik's JSX transform via the react-jsx preset pointing to @builder.io/qwik. The Qwik Vite plugin handles the optimization and references tsconfig.json for linting/ESLint during development. Generate a new Qwik project with npm create qwik@latest to get the exact current starter tsconfig.json.
Citations:
- 1: https://github.com/fbernack/qwik-tutorial/blob/main/tsconfig.json
- 2: https://qwik.dev/docs/advanced/vite
- 3: https://github.com/QwikDev/qwik-build/blob/main/tsconfig.json
- 4: https://github.com/BuilderIO/builder-qwik-example/blob/main/tsconfig.json
- 5: https://qwik.dev/docs/project-structure/
- 6: https://qwik.dev/docs/getting-started
- 7: https://qwik.builder.io/docs/integrations/react/
- 8: https://qwik.builder.io/docs/components/rendering/
- 9: fix(tsconfig.json): add DOM.Iterable to compilerOptions of tsconfig.json QwikDev/qwik#1371
🏁 Script executed:
# Check if tsconfig.json exists and read it
if [ -f "tsconfig.json" ]; then
echo "=== tsconfig.json exists ==="
cat -n tsconfig.json
else
echo "=== tsconfig.json does NOT exist ==="
fiRepository: AOSSIE-Org/SocialShareButton
Length of output: 108
🏁 Script executed:
# Read package.json to check dependencies and devDependencies
cat -n package.json | head -100Repository: AOSSIE-Org/SocialShareButton
Length of output: 1868
🏁 Script executed:
# Search for JSX configuration in the codebase
rg "jsxImportSource|jsx.*react" --type ts --type tsx --type jsonRepository: AOSSIE-Org/SocialShareButton
Length of output: 99
🏁 Script executed:
# Search for `@builder.io/qwik` imports and usage
rg "@builder.io/qwik|qwik" --type js --type jsxRepository: AOSSIE-Org/SocialShareButton
Length of output: 99
🏁 Script executed:
# Check the React wrapper file
cat -n src/social-share-button-react.jsxRepository: AOSSIE-Org/SocialShareButton
Length of output: 3967
🏁 Script executed:
# Look for all files that use JSX
fd "\.(jsx|tsx)$" src/Repository: AOSSIE-Org/SocialShareButton
Length of output: 138
🏁 Script executed:
# Check the Qwik wrapper file
cat -n src/social-share-button-qwik.tsxRepository: AOSSIE-Org/SocialShareButton
Length of output: 2249
🏁 Script executed:
# Search for `@builder.io/qwik` usage without relying on file type
rg "@builder.io/qwik|from.*qwik"Repository: AOSSIE-Org/SocialShareButton
Length of output: 731
🏁 Script executed:
# Check if there's any TypeScript configuration elsewhere (tsconfig, tsc, build scripts)
rg "tsconfig|typescript" package.jsonRepository: AOSSIE-Org/SocialShareButton
Length of output: 54
Add tsconfig.json to configure Qwik JSX namespace.
The project includes both @types/react and a Qwik wrapper file (src/social-share-button-qwik.tsx) that uses Qwik JSX. Without a tsconfig.json specifying the JSX configuration, TypeScript will default to React's JSX namespace, causing type conflicts in the .tsx file.
Create a tsconfig.json with:
{
"compilerOptions": {
"jsx": "react-jsx",
"jsxImportSource": "@builder.io/qwik"
}
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@package.json` at line 45, TypeScript is defaulting to React's JSX namespace
due to the presence of "@types/react", causing type conflicts in the Qwik
component (e.g., social-share-button-qwik.tsx); add a tsconfig.json at the
repository root with compilerOptions that set "jsx" to "react-jsx" and
"jsxImportSource" to "@builder.io/qwik" so the Qwik JSX namespace is used for
.tsx files, then commit that tsconfig.json to the repo.
| "dependencies": { | ||
| "@builder.io/qwik": "^1.19.2" | ||
| } |
There was a problem hiding this comment.
Runtime dependency contradicts "Zero Dependencies" design goal.
Adding @builder.io/qwik as a runtime dependency means all users of this package will install Qwik, even if they don't use the Qwik wrapper. This contradicts the project's core "Zero Dependencies" value proposition shown in the README badges.
The Qwik package should be a peerDependency instead, making it optional for consumers who want Qwik support while keeping the library dependency-free for vanilla JS, React, and Vue users.
🔧 Proposed fix: Move to peerDependencies
- "dependencies": {
- "@builder.io/qwik": "^1.19.2"
- }
+ "peerDependencies": {
+ "@builder.io/qwik": "^1.19.2"
+ },
+ "peerDependenciesMeta": {
+ "@builder.io/qwik": {
+ "optional": true
+ }
+ }📝 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.
| "dependencies": { | |
| "@builder.io/qwik": "^1.19.2" | |
| } | |
| "peerDependencies": { | |
| "@builder.io/qwik": "^1.19.2" | |
| }, | |
| "peerDependenciesMeta": { | |
| "@builder.io/qwik": { | |
| "optional": true | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@package.json` around lines 51 - 53, The package currently lists
"@builder.io/qwik" under "dependencies" which forces consumers to install Qwik;
move "@builder.io/qwik" from the "dependencies" object into "peerDependencies"
(and optionally add it to "devDependencies" for local tests) so Qwik is optional
for consumers while preserving the project's zero-deps design; update the
package.json entries accordingly and run npm/yarn install to validate the
change.
| onShare?: QRL<() => void>; | ||
| onCopy?: QRL<() => void>; |
There was a problem hiding this comment.
onShare and onCopy callbacks are defined but never passed to the constructor.
The Props interface accepts onShare and onCopy callbacks (lines 16-17), but they are not included in the options object passed to new window.SocialShareButton() (lines 33-54). Users expecting these callbacks to work will experience silent failures.
🔧 Proposed fix: Wire callbacks to constructor
const shareButton = new window.SocialShareButton({
container: container.value,
url: props.url || window.location.href,
title: props.title || document.title,
description: props.description || '',
hashtags: props.hashtags || [],
via: props.via || '',
platforms:
props.platforms || [
'whatsapp',
'facebook',
'twitter',
'linkedin',
'telegram',
'reddit',
+ 'pinterest',
],
theme: props.theme || 'dark',
buttonText: props.buttonText || 'Share',
customClass: props.customClass || '',
buttonStyle: props.buttonStyle || 'default',
modalPosition: props.modalPosition || 'center',
+ onShare: props.onShare,
+ onCopy: props.onCopy,
});Also applies to: 31-54
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/social-share-button-qwik.tsx` around lines 16 - 17, Props interface
defines onShare and onCopy but those callbacks aren't forwarded into the options
for new window.SocialShareButton; update the SocialShareButton instantiation to
include onShare: props.onShare and onCopy: props.onCopy (or the local variables
holding those QRLs) in the options object passed to the constructor so the
SocialShareButton instance receives and can invoke them.
| export const SocialShareButton = component$<Props>((props) => { | ||
| // Use null instead of undefined to satisfy ref typing | ||
| const container = useSignal<HTMLDivElement | null>(null); | ||
|
|
||
| useVisibleTask$(({ cleanup }) => { | ||
| if (typeof window !== 'undefined' && window.SocialShareButton && container.value) { | ||
| const shareButton = new window.SocialShareButton({ | ||
| container: container.value, | ||
| url: props.url || window.location.href, | ||
| title: props.title || document.title, | ||
| description: props.description || '', | ||
| hashtags: props.hashtags || [], | ||
| via: props.via || '', | ||
| platforms: | ||
| props.platforms || [ | ||
| 'whatsapp', | ||
| 'facebook', | ||
| 'twitter', | ||
| 'linkedin', | ||
| 'telegram', | ||
| 'reddit', | ||
| ], | ||
| theme: props.theme || 'dark', | ||
| buttonText: props.buttonText || 'Share', | ||
| customClass: props.customClass || '', | ||
| buttonStyle: props.buttonStyle || 'default', | ||
| modalPosition: props.modalPosition || 'center', | ||
| }); | ||
|
|
||
| cleanup(() => shareButton.destroy()); | ||
| } | ||
| }); | ||
|
|
||
| return <div ref={container}></div>; | ||
| }); No newline at end of file |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Add inline comments to the component and task function.
Per repository guidelines, library source files should have proper inline comments explaining what functions do, why they exist, and their parameters/return values.
📝 Proposed documentation improvements
+/**
+ * Qwik wrapper component for SocialShareButton.
+ * Initializes the vanilla SocialShareButton library only on the client side
+ * using useVisibleTask$ to remain SSR-safe and compatible with Qwik's resumability model.
+ *
+ * `@param` props - Configuration options for the share button (url, title, theme, etc.)
+ * `@returns` A div element that serves as the container for the share button
+ */
export const SocialShareButton = component$<Props>((props) => {
- // Use null instead of undefined to satisfy ref typing
+ // Container ref - initialized to null, populated when div mounts
const container = useSignal<HTMLDivElement | null>(null);
+ // useVisibleTask$ runs only on the client when the component becomes visible,
+ // ensuring browser APIs (window, document) are safely accessible
useVisibleTask$(({ cleanup }) => {
if (typeof window !== 'undefined' && window.SocialShareButton && container.value) {
const shareButton = new window.SocialShareButton({
// ... options
});
+ // Register cleanup to destroy the instance and prevent memory leaks
+ // when the component unmounts or the task re-runs
cleanup(() => shareButton.destroy());
}
});
return <div ref={container}></div>;
});Based on learnings: flag any newly added or modified function that lacks a descriptive comment explaining what it does, why it exists, and its parameters/return values.
📝 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.
| export const SocialShareButton = component$<Props>((props) => { | |
| // Use null instead of undefined to satisfy ref typing | |
| const container = useSignal<HTMLDivElement | null>(null); | |
| useVisibleTask$(({ cleanup }) => { | |
| if (typeof window !== 'undefined' && window.SocialShareButton && container.value) { | |
| const shareButton = new window.SocialShareButton({ | |
| container: container.value, | |
| url: props.url || window.location.href, | |
| title: props.title || document.title, | |
| description: props.description || '', | |
| hashtags: props.hashtags || [], | |
| via: props.via || '', | |
| platforms: | |
| props.platforms || [ | |
| 'whatsapp', | |
| 'facebook', | |
| 'twitter', | |
| 'linkedin', | |
| 'telegram', | |
| 'reddit', | |
| ], | |
| theme: props.theme || 'dark', | |
| buttonText: props.buttonText || 'Share', | |
| customClass: props.customClass || '', | |
| buttonStyle: props.buttonStyle || 'default', | |
| modalPosition: props.modalPosition || 'center', | |
| }); | |
| cleanup(() => shareButton.destroy()); | |
| } | |
| }); | |
| return <div ref={container}></div>; | |
| }); | |
| /** | |
| * Qwik wrapper component for SocialShareButton. | |
| * Initializes the vanilla SocialShareButton library only on the client side | |
| * using useVisibleTask$ to remain SSR-safe and compatible with Qwik's resumability model. | |
| * | |
| * `@param` props - Configuration options for the share button (url, title, theme, etc.) | |
| * `@returns` A div element that serves as the container for the share button | |
| */ | |
| export const SocialShareButton = component$<Props>((props) => { | |
| // Container ref - initialized to null, populated when div mounts | |
| const container = useSignal<HTMLDivElement | null>(null); | |
| // useVisibleTask$ runs only on the client when the component becomes visible, | |
| // ensuring browser APIs (window, document) are safely accessible | |
| useVisibleTask$(({ cleanup }) => { | |
| if (typeof window !== 'undefined' && window.SocialShareButton && container.value) { | |
| const shareButton = new window.SocialShareButton({ | |
| container: container.value, | |
| url: props.url || window.location.href, | |
| title: props.title || document.title, | |
| description: props.description || '', | |
| hashtags: props.hashtags || [], | |
| via: props.via || '', | |
| platforms: | |
| props.platforms || [ | |
| 'whatsapp', | |
| 'facebook', | |
| 'twitter', | |
| 'linkedin', | |
| 'telegram', | |
| 'reddit', | |
| ], | |
| theme: props.theme || 'dark', | |
| buttonText: props.buttonText || 'Share', | |
| customClass: props.customClass || '', | |
| buttonStyle: props.buttonStyle || 'default', | |
| modalPosition: props.modalPosition || 'center', | |
| }); | |
| // Register cleanup to destroy the instance and prevent memory leaks | |
| // when the component unmounts or the task re-runs | |
| cleanup(() => shareButton.destroy()); | |
| } | |
| }); | |
| return <div ref={container}></div>; | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/social-share-button-qwik.tsx` around lines 27 - 61, Add inline
JSDoc-style comments above the SocialShareButton component and inside the
useVisibleTask$ callback: describe what the SocialShareButton component does,
list its Props (url, title, description, hashtags, via, platforms, theme,
buttonText, customClass, buttonStyle, modalPosition) and its return value (a div
container ref), and explain the useVisibleTask$ logic (checks for
window.SocialShareButton, initializes the external widget via new
window.SocialShareButton with the mapped props, and registers cleanup by calling
shareButton.destroy()). Also comment the container signal (container:
useSignal<HTMLDivElement | null>) to note why null is used and its role as the
widget mount point.
| platforms: | ||
| props.platforms || [ | ||
| 'whatsapp', | ||
| 'facebook', | ||
| 'twitter', | ||
| 'linkedin', | ||
| 'telegram', | ||
| 'reddit', | ||
| ], |
There was a problem hiding this comment.
Default platforms list is missing 'pinterest' compared to core library.
The core SocialShareButton constructor (in src/social-share-button.js) includes 'pinterest' in the default platforms array, but this Qwik wrapper omits it. This creates an inconsistent experience between vanilla JS and Qwik users.
🔧 Proposed fix
platforms:
props.platforms || [
'whatsapp',
'facebook',
'twitter',
'linkedin',
'telegram',
'reddit',
+ 'pinterest',
],📝 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.
| platforms: | |
| props.platforms || [ | |
| 'whatsapp', | |
| 'facebook', | |
| 'twitter', | |
| 'linkedin', | |
| 'telegram', | |
| 'reddit', | |
| ], | |
| platforms: | |
| props.platforms || [ | |
| 'whatsapp', | |
| 'facebook', | |
| 'twitter', | |
| 'linkedin', | |
| 'telegram', | |
| 'reddit', | |
| 'pinterest', | |
| ], |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/social-share-button-qwik.tsx` around lines 40 - 48, The default platforms
array in the Qwik wrapper is missing 'pinterest', causing inconsistency with the
core SocialShareButton; update the default value for props.platforms in
src/social-share-button-qwik.tsx (the object where platforms: props.platforms ||
[...]) to include 'pinterest' alongside
'whatsapp','facebook','twitter','linkedin','telegram','reddit' so the Qwik
SocialShareButton mirrors the core SocialShareButton defaults.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
index.html (2)
474-491:⚠️ Potential issue | 🔴 CriticalRemove the top-level Qwik hook demo; it cannot run in a plain
index.html.
useVisibleTask$must run inside acomponent$, so calling it at module scope will throw before any demo renders. This block also targets#demo-qwik, but the section above never creates that container. For this static page, either drop the live Qwik script entirely or replace it with a vanillaSocialShareButtoninitializer against a real demo container. The CDN pin (1.0.0) is also out of sync with the repo's declared@builder.io/qwikversion (^1.19.2).Qwik docs: can `useVisibleTask$` be called at module scope outside `component$`, and is loading `@builder.io/qwik@1.0.0` from a CDN consistent with a project dependency of `@builder.io/qwik@^1.19.2`?🧹 Safer immediate fix
-<!-- Load Qwik for the demo --> -<script type="module"> - import { component$, useVisibleTask$, useSignal } from 'https://cdn.jsdelivr.net/npm/@builder.io/qwik@1.0.0/dist/qwik.mjs'; - // Demo - useVisibleTask$(() => { - const container = document.querySelector('#demo-qwik'); - if (container && window.SocialShareButton) { - new window.SocialShareButton({ - container, - url: 'https://github.com/AOSSIE-Org/SocialShareButton', - title: 'SocialShareButton - Qwik Demo', - description: 'Demo of SocialShareButton in Qwik', - buttonText: 'Share', - theme: 'dark' - }); - } - }); -</script>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@index.html` around lines 474 - 491, The Qwik demo uses useVisibleTask$ at module scope which is invalid and will throw; remove or relocate this demo code: either delete the top-level import and useVisibleTask$ block, or wrap the logic inside a proper component$ (using component$ and calling useVisibleTask$ inside it) and ensure the demo actually creates a container with id '#demo-qwik' before instantiating new window.SocialShareButton; also update the CDN import to match the project's Qwik version (replace the hard-coded '@builder.io/qwik@1.0.0' import with a matching version or remove the CDN import and use a vanilla initializer for SocialShareButton if you want a plain index.html demo).
456-456:⚠️ Potential issue | 🟡 MinorSet
type="button"on the new copy button.Without it, this element defaults to
submit, which is the HTML lint warning already reported here and can cause accidental form submission if this block is ever embedded in a form.🔧 Proposed fix
- <button class="copy-btn" aria-label="Copy code">Copy</button> + <button type="button" class="copy-btn" aria-label="Copy code">Copy</button>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@index.html` at line 456, The copy button element (class "copy-btn") lacks an explicit type, causing it to default to "submit" and risk accidental form submissions; update the <button class="copy-btn" ...> markup to include type="button" so the element will act as a non-submitting control (locate the button with class "copy-btn" and add the type attribute).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@index.html`:
- Around line 446-453: Add a Step 1 after the intro paragraph in the Qwik
Integration demo that explains how to acquire the Qwik wrapper: state whether to
copy the new source file src/social-share-button-qwik.tsx into their project,
install a forthcoming npm package, or wait for the next release; specifically
reference the import path ~/components/social-share-button-qwik so readers know
where to place or import the file, and clearly indicate which option is current
(e.g., "copy file from repo" if not yet published) and any command or path they
should use.
---
Duplicate comments:
In `@index.html`:
- Around line 474-491: The Qwik demo uses useVisibleTask$ at module scope which
is invalid and will throw; remove or relocate this demo code: either delete the
top-level import and useVisibleTask$ block, or wrap the logic inside a proper
component$ (using component$ and calling useVisibleTask$ inside it) and ensure
the demo actually creates a container with id '#demo-qwik' before instantiating
new window.SocialShareButton; also update the CDN import to match the project's
Qwik version (replace the hard-coded '@builder.io/qwik@1.0.0' import with a
matching version or remove the CDN import and use a vanilla initializer for
SocialShareButton if you want a plain index.html demo).
- Line 456: The copy button element (class "copy-btn") lacks an explicit type,
causing it to default to "submit" and risk accidental form submissions; update
the <button class="copy-btn" ...> markup to include type="button" so the element
will act as a non-submitting control (locate the button with class "copy-btn"
and add the type attribute).
🪄 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: 457dbc44-cf30-4f5d-b3e0-7ad1ea68ac19
📒 Files selected for processing (2)
README.mdindex.html
| <!-- Qwik Integration Demo Section --> | ||
| <div class="demo-section" > | ||
| <h2>⚡ Qwik / QwikCity Integration</h2> | ||
| <p>Use SocialShareButton with the Qwik framework, fully SSR-safe and client-only.</p> | ||
|
|
||
| <div class="demo-grid"> | ||
|
|
||
| </div> |
There was a problem hiding this comment.
Add the missing acquisition step for the Qwik wrapper.
After the intro paragraph, this section only leaves an empty placeholder before the usage snippet, so readers still are not told how ~/components/social-share-button-qwik gets into their app. Because the existing CDN examples in this repo predate this new wrapper, the Qwik guide is not actionable yet. Please add a Step 1 that explicitly tells users whether to copy src/social-share-button-qwik.tsx, install it from npm, or wait for the next tagged release.
Based on learnings: new wrapper files added in a PR are not part of any CDN release until the next version tag is cut, so docs need an explicit acquisition path for the new file.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@index.html` around lines 446 - 453, Add a Step 1 after the intro paragraph in
the Qwik Integration demo that explains how to acquire the Qwik wrapper: state
whether to copy the new source file src/social-share-button-qwik.tsx into their
project, install a forthcoming npm package, or wait for the next release;
specifically reference the import path ~/components/social-share-button-qwik so
readers know where to place or import the file, and clearly indicate which
option is current (e.g., "copy file from repo" if not yet published) and any
command or path they should use.
|
Thanks you for your feedback . I will update everything within 2 days |
|
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: |
1 participant
Addressed Issues:
Fixes #57
This PR adds Qwik support for the SocialShareButton component by introducing a lightweight wrapper compatible with Qwik's resumability model and SSR-first architecture.
Screenshots/Recordings:
Additional Notes:
Now we have qwik integration along with react js .
Checklist
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.
Check one of the checkboxes below:
I have used the following AI models and tools: TODO
Summary by CodeRabbit
New Features
Documentation
Chores