feat: Add Auto System Theme detection and Floating Share Bar.#152
feat: Add Auto System Theme detection and Floating Share Bar.#152sejalkutriyar wants to merge 3 commits into
Conversation
github-actions
Bot
added
configuration
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 51 minutes and 13 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (3)
WalkthroughThe PR adds floating display mode and auto theme detection to the social share button component, alongside extensive formatting and whitespace normalization across documentation, configuration, and source files. New CSS styles support floating wrapper positioning; JavaScript enhancements enable Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Browser/Client
participant SSB as SocialShareButton
participant DOM as DOM
participant Modal as Modal Instance
participant Media as matchMedia API
Client->>SSB: Initialize with theme:'auto'<br/>displayMode:'floating'
SSB->>Media: matchMedia('(prefers-color-scheme: dark)')
Media-->>SSB: Initial theme state
SSB->>DOM: Create floating wrapper element
SSB->>DOM: Append wrapper to document.body
SSB->>Modal: createModal() with resolved theme
Modal-->>SSB: Modal instance created
SSB->>Media: Register listener for theme changes
Media->>SSB: OS theme changes (e.g., light→dark)
SSB->>Modal: Update theme classes dynamically
Client->>SSB: destroy()
SSB->>Media: Unregister matchMedia listener
SSB->>DOM: Remove floating wrapper from DOM
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 |
github-actions
Bot
added
first-time-contributor
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
docs/Roadmap.md (1)
1-489: 🧹 Nitpick | 🔵 TrivialConsider separating formatting changes from feature work.
This file contains only cosmetic changes (table spacing, emphasis markers, code quote style, whitespace normalization) that are unrelated to the PR's stated objectives (Auto System Theme and Floating Share Bar). While these formatting tweaks are harmless, mixing them with feature work makes the PR diff harder to review and dilutes the git history.
For future PRs, consider keeping formatting/style cleanup in separate commits or PRs from functional changes.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/Roadmap.md` around lines 1 - 489, The PR mixes unrelated cosmetic edits in Roadmap.md (table spacing, emphasis/quote style, whitespace normalization across sections like "Phase 0" and the top-level headings) with the functional work (Auto System Theme and Floating Share Bar); separate these formatting-only changes into their own commit or PR: create a new branch/commit that contains only the Markdown/style edits for Roadmap.md (or revert them here), leave this PR with only the functional changes, and update the PR title/description to state clearly which commit/PR contains formatting vs feature work so reviewers can review diffs for symbols like "Phase 0", "Target Architecture" and the Roadmap tables independently.src/social-share-button.js (1)
91-107:⚠️ Potential issue | 🟡 Minor
floatingPositionis not validated — typos produce a broken CSS class.
wrapper.className = \social-share-floating-wrapper ${this.options.floatingPosition}`blindly concatenates user input. If a consumer passes"bottomRight","BOTTOM-RIGHT", or any unknown value, no corner positioning rule matches and the wrapper will render at the viewport origin with only the baseposition: fixedrule. Consider validating against the allowed set (bottom-right | bottom-left | top-right | top-left`) and falling back to a default.♻️ Suggested fix
+ const validPositions = ["bottom-right", "bottom-left", "top-right", "top-left"]; + const position = validPositions.includes(this.options.floatingPosition) + ? this.options.floatingPosition + : "bottom-right"; if (this.options.displayMode === "floating" && typeof document !== "undefined") { const wrapper = document.createElement("div"); - wrapper.className = `social-share-floating-wrapper ${this.options.floatingPosition}`; + wrapper.className = `social-share-floating-wrapper ${position}`;🤖 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 91 - 107, Validate and normalize this.options.floatingPosition before using it in wrapper.className: ensure the value comes from the allowed set ("bottom-right", "bottom-left", "top-right", "top-left") (normalize common variants like "bottomRight" or uppercase by mapping or lowercasing and inserting hyphen) and fall back to a default (e.g., "bottom-right") when invalid or missing; update the code around displayMode === "floating" where wrapper.className is set (reference symbols: this.options.floatingPosition, wrapper.className, this.floatingWrapper) to compute a validatedPosition and use that in the class string.src/social-share-button-preact.jsx (1)
12-36:⚠️ Potential issue | 🟠 MajorNew
displayMode/floatingPositionoptions not exposed through the Preact wrapper.The PR adds
displayMode: 'floating'andfloatingPositionto the core library, but this wrapper's destructured props list does not include them, nor are they forwarded tonew window.SocialShareButton(...)at line 92 or toupdateOptions(...)at line 147. Preact consumers have no way to opt into the floating share bar feature advertised in this PR.♻️ Proposed fix
buttonStyle = "default", modalPosition = "center", + displayMode = "inline", + floatingPosition = "bottom-right", // Analytics props — the library emits events but never collects data itself.Also add them to both
latestOptionsRef.currentand theupdateOptionscall plus the dependency array of the update effect.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/social-share-button-preact.jsx` around lines 12 - 36, The Preact wrapper SocialShareButton is missing the new displayMode and floatingPosition props so consumers cannot use the floating share bar; add displayMode and floatingPosition to the component's destructured props with sensible defaults, ensure they are included in latestOptionsRef.current when building the options passed to new window.SocialShareButton(...) and also passed into updateOptions(...) in the effect, and include them in the effect dependency array so updates to displayMode/floatingPosition reconfigure the instance.src/social-share-button-react.jsx (1)
10-71:⚠️ Potential issue | 🟠 MajorNew
displayMode/floatingPositionoptions not exposed through the React wrapper.Same gap as the Preact wrapper: the props destructuring and the options passed into
new window.SocialShareButton(...)at lines 51-71 (and theupdateOptionscall at lines 93-112) do not include the newdisplayModeandfloatingPositionoptions introduced insrc/social-share-button.js. React consumers cannot opt into the Floating Share Bar feature this PR adds.♻️ Proposed fix
buttonStyle = "default", modalPosition = "center", + displayMode = "inline", + floatingPosition = "bottom-right",Forward both in the
new window.SocialShareButton({...})call and inupdateOptions({...}), plus add them to the second effect's dependency array.🤖 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 10 - 71, The React wrapper is not forwarding the new displayMode and floatingPosition options; add displayMode and floatingPosition to the SocialShareButton component props (with appropriate defaults), include them in the object passed to new window.SocialShareButton({...}) when creating shareButtonRef.current, include them in the options passed to shareButtonRef.current.updateOptions({...}) in the update effect, and add them to that effect's dependency array so updates to these props reconfigure the vanilla instance; reference the SocialShareButton React component, containerRef, shareButtonRef, new window.SocialShareButton({...}), and updateOptions({...}) when making these changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/Roadmap.md`:
- Around line 30-46: Update the Roadmap.md "What Already Exists" table to
include the new theme option and floating display mode: add `theme: 'auto'`
(OS-level detection) alongside `theme: 'dark' | 'light'` and add a row for
`displayMode: 'floating'` with a note about the `floatingPosition`
configuration; also update any Notes cell or add a brief remark under that
section that these capabilities were introduced in this PR so the roadmap
accurately reflects `theme` and `displayMode` features (search for the existing
table and the literals `theme: 'dark' | 'light'`, `displayMode`, and
`floatingPosition` to locate the lines to change).
In `@index.html`:
- Around line 410-414: The "Check the bottom right of your screen!" hint under
the "Floating Share Bar" heading is misleading because the floating demo is
missing its required container during initialization (the floating demo
initialization references a missing container). Remove or replace that specific
hint paragraph (the <p> whose inline style contains "font-size: 11px; opacity:
0.8; margin-top: 5px" and the text "Check the bottom right of your screen!")
with a neutral note such as "Floating demo disabled until configuration is
completed" or hide it until the floating demo's container is correctly provided
in the floating demo initialization code.
- Around line 505-536: The CDN URLs in the HTML code block reference a
non-existent tag "@v1.0.4", causing 404s; update the two jsDelivr references
(the href and script src that point to social-share-button.css and
social-share-button.js) to a valid tag such as "@main" or "@latest" (or the next
released tag) so the Qwik wrapper file social-share-button-qwik.tsx and assets
load correctly; keep the rest of the example (SocialShareButton component usage)
unchanged.
- Around line 718-727: The Floating Share Bar demo fails because
SocialShareButton's constructor only calls init() when this.options.container
exists; update the constructor logic in the SocialShareButton class so it also
auto-initializes when options.displayMode === "floating" (i.e., call this.init()
if this.options.container || this.options.displayMode === "floating"), and
ensure init()/createButton() will create and append the floating wrapper to
document.body when displayMode is "floating". Reference: SocialShareButton
constructor, init(), createButton(), and options.displayMode.
In `@src/social-share-button.js`:
- Around line 109-135: The modal's "auto" theme handling is only set in
createModal(), so updateOptions({ theme }) won't register or update the
matchMedia listener or the modal's class; modify updateOptions to detect when
this.options.theme changes (to or from "auto"), and when it does: if new theme
=== "auto" call the same setup logic used in createModal() to resolve
resolvedTheme, register themeMediaQuery and themeChangeHandler (store them on
this.themeMediaQuery / this.themeChangeHandler), and update this.modal.classList
accordingly; if changing away from "auto", remove the listener by calling
this.themeMediaQuery.removeEventListener("change", this.themeChangeHandler) and
clear stored refs, and update this.modal.classList to the explicit theme. Ensure
createModal and updateOptions share or reuse the theme setup/teardown logic so
behavior is consistent.
---
Outside diff comments:
In `@docs/Roadmap.md`:
- Around line 1-489: The PR mixes unrelated cosmetic edits in Roadmap.md (table
spacing, emphasis/quote style, whitespace normalization across sections like
"Phase 0" and the top-level headings) with the functional work (Auto System
Theme and Floating Share Bar); separate these formatting-only changes into their
own commit or PR: create a new branch/commit that contains only the
Markdown/style edits for Roadmap.md (or revert them here), leave this PR with
only the functional changes, and update the PR title/description to state
clearly which commit/PR contains formatting vs feature work so reviewers can
review diffs for symbols like "Phase 0", "Target Architecture" and the Roadmap
tables independently.
In `@src/social-share-button-preact.jsx`:
- Around line 12-36: The Preact wrapper SocialShareButton is missing the new
displayMode and floatingPosition props so consumers cannot use the floating
share bar; add displayMode and floatingPosition to the component's destructured
props with sensible defaults, ensure they are included in
latestOptionsRef.current when building the options passed to new
window.SocialShareButton(...) and also passed into updateOptions(...) in the
effect, and include them in the effect dependency array so updates to
displayMode/floatingPosition reconfigure the instance.
In `@src/social-share-button-react.jsx`:
- Around line 10-71: The React wrapper is not forwarding the new displayMode and
floatingPosition options; add displayMode and floatingPosition to the
SocialShareButton component props (with appropriate defaults), include them in
the object passed to new window.SocialShareButton({...}) when creating
shareButtonRef.current, include them in the options passed to
shareButtonRef.current.updateOptions({...}) in the update effect, and add them
to that effect's dependency array so updates to these props reconfigure the
vanilla instance; reference the SocialShareButton React component, containerRef,
shareButtonRef, new window.SocialShareButton({...}), and updateOptions({...})
when making these changes.
In `@src/social-share-button.js`:
- Around line 91-107: Validate and normalize this.options.floatingPosition
before using it in wrapper.className: ensure the value comes from the allowed
set ("bottom-right", "bottom-left", "top-right", "top-left") (normalize common
variants like "bottomRight" or uppercase by mapping or lowercasing and inserting
hyphen) and fall back to a default (e.g., "bottom-right") when invalid or
missing; update the code around displayMode === "floating" where
wrapper.className is set (reference symbols: this.options.floatingPosition,
wrapper.className, this.floatingWrapper) to compute a validatedPosition and use
that in the class string.
🪄 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: a542d4a7-22ff-4d2e-a166-0f83aa92de7b
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (12)
CONTRIBUTING.mdREADME.mddocs/Roadmap.mdeslint.config.jsindex.htmllanding-page/README.mdlanding-page/src/app/globals.csssrc/social-share-analytics.jssrc/social-share-button-preact.jsxsrc/social-share-button-react.jsxsrc/social-share-button.csssrc/social-share-button.js
💤 Files with no reviewable changes (1)
- README.md
| <h3>Floating Share Bar</h3> | ||
| <p>Pins the share button to the screen corner.</p> | ||
| <p style="font-size: 11px; opacity: 0.8; margin-top: 5px"> | ||
| Check the bottom right of your screen! | ||
| </p> |
There was a problem hiding this comment.
Misleading hint — floating button won't actually appear.
Due to the missing container in the floating demo initialization (see comment on lines 718-727), the "Check the bottom right of your screen!" instruction will confuse users. This will be resolved automatically once the floating demo is fixed.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@index.html` around lines 410 - 414, The "Check the bottom right of your
screen!" hint under the "Floating Share Bar" heading is misleading because the
floating demo is missing its required container during initialization (the
floating demo initialization references a missing container). Remove or replace
that specific hint paragraph (the <p> whose inline style contains "font-size:
11px; opacity: 0.8; margin-top: 5px" and the text "Check the bottom right of
your screen!") with a neutral note such as "Floating demo disabled until
configuration is completed" or hide it until the floating demo's container is
correctly provided in the floating demo initialization code.
…eme reactivity, and CDN versions
github-actions
Bot
added
no-issue-linked
|
Hello 👋 This PR has had no activity for more than 2 weeks. If you are still working on it, please push an update or leave a comment. Ping a maintainer if you believe it is ready for review or merge! This PR will be automatically closed in 7 days if there is no further activity. |
1 participant
Addressed Issues:
This PR introduces two highly requested UX feature enhancements to standard share buttons without adding any external dependencies: Auto OS-level Theme Detection and an option for a Floating Action Share Bar. It also replaces the previous closed PR (#140).
Key Changes
theme: 'auto'capability that detects user OS preference (Light/Dark mode) usingwindow.matchMedia('(prefers-color-scheme: light)')and auto-adapts the modal UI class. It seamlessly listens for live OS preference changes while keeping the modal reactive.displayMode: 'floating'alongside the standard inline configuration. When enabled, the button creates its own wrapper appended directly todocument.bodywithposition: fixedand gracefully floats in corners based onfloatingPosition.index.htmlto be multi-line and easily readable..containerwrapper to fix the broken layout.>character in the Qwik code block.transform-originproperties fortop-rightandtop-leftfloating modes in mobile media queries.Checklist
Summary by CodeRabbit
New Features
Style
Chores