Conversation
Adds the Calendly booking widget to the registry as `useScriptCalendly`, covering inline, popup, and badge embeds with first-party proxy support for `assets.calendly.com` and Partytown forwards for the widget initialisers.
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
commit: |
|
ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
✅ Files skipped from review due to trivial changes (3)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThis pull request introduces Calendly scheduling widget integration to Nuxt Scripts. The implementation provides a useScriptCalendly() composable that handles loading the Calendly external widget script with optional bundling and proxying, injects the widget stylesheet, and exposes a typed proxy wrapping window.Calendly methods. The integration includes widget variants for inline, popup, and badge embeds with support for prefill and UTM tracking options. A stub queue mechanism queues method calls until the SDK loads. The PR adds user documentation, playground demonstrations, comprehensive E2E test coverage for both bundled and CDN delivery modes, unit tests validating the stub queue and proxy configuration, and privacy tier declarations marking Calendly as PRIVACY_IP_ONLY with domain-level IP anonymization. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
FIRST_PARTY.md (1)
116-148:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winClarify Calendly’s iframe exception in the table.
Current wording can imply all Calendly traffic is first-party proxied, but the booking iframe remains direct to
calendly.com. Add a brief note to avoid privacy-scope confusion.✏️ Suggested doc clarification
-| `calendly` | calendly | `PRIVACY_IP_ONLY` | Path A | +| `calendly` | calendly | `PRIVACY_IP_ONLY` | Path A (script asset proxying; booking iframe remains direct to calendly.com) |🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@FIRST_PARTY.md` around lines 116 - 148, The table entry for the `calendly` config key (row showing `calendly` | `PRIVACY_IP_ONLY` | Path A) is ambiguous about Calendly’s iframe being proxied; update that row to add a concise clarifying note that while general Calendly integrations are treated as PRIVACY_IP_ONLY via Path A, the booking iframe loads directly from calendly.com and is not proxied (e.g., append “— booking iframe loads directly from calendly.com, not proxied” or similar) so readers don’t assume full first‑party proxy coverage.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/script/src/runtime/registry/calendly.ts`:
- Around line 34-40: The CalendlyInlineWidgetOptions interface currently types
parentElement as HTMLElement | string which allows invalid selector strings;
change parentElement to only HTMLElement in the CalendlyInlineWidgetOptions
declaration so callers must pass a DOM element, and update any call sites or
helper functions that constructed or accepted selector strings to instead
resolve the element (e.g., querySelector) before passing it to the
CalendlyInlineWidgetOptions constructor or to the functions that consume
parentElement; ensure references to parentElement in functions like the inline
widget renderer treat it as an HTMLElement (no string handling) and add runtime
checks where appropriate to throw a clear error if a non-HTMLElement is
supplied.
---
Outside diff comments:
In `@FIRST_PARTY.md`:
- Around line 116-148: The table entry for the `calendly` config key (row
showing `calendly` | `PRIVACY_IP_ONLY` | Path A) is ambiguous about Calendly’s
iframe being proxied; update that row to add a concise clarifying note that
while general Calendly integrations are treated as PRIVACY_IP_ONLY via Path A,
the booking iframe loads directly from calendly.com and is not proxied (e.g.,
append “— booking iframe loads directly from calendly.com, not proxied” or
similar) so readers don’t assume full first‑party proxy coverage.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3e0facc9-42af-4d83-bbe3-70f9832d2578
📒 Files selected for processing (25)
FIRST_PARTY.mddocs/content/docs/1.guides/2.first-party.mddocs/content/scripts/calendly.mdpackages/script/src/registry-logos.tspackages/script/src/registry-types.jsonpackages/script/src/registry.tspackages/script/src/runtime/registry/calendly.tspackages/script/src/runtime/registry/schemas.tspackages/script/src/runtime/types.tspackages/script/src/script-meta.tsplayground/pages/index.vueplayground/pages/third-parties/calendly/default.vueplayground/pages/third-parties/calendly/nuxt-scripts.vuetest/e2e/_calendly-suite.tstest/e2e/calendly-cdn.test.tstest/e2e/calendly.test.tstest/fixtures/calendly-cdn/nuxt.config.tstest/fixtures/calendly-cdn/tsconfig.jsontest/fixtures/calendly/app.vuetest/fixtures/calendly/nuxt.config.tstest/fixtures/calendly/package.jsontest/fixtures/calendly/pages/index.vuetest/fixtures/calendly/tsconfig.jsontest/types/types.test-d.tstest/unit/proxy-configs.test.ts
The calendly e2e tests failed in CI because: - The fixtures lacked a `prepare:fixtures` entry, so `.nuxt/tsconfig.json` was missing when Vite parsed the page that imports `useScriptCalendly`. - The cdn fixture had no per-call `bundle: false`, so the script was always served from /_scripts/assets/ (proxy) instead of the CDN. - The stub queue e2e was racing the real script load (`onNuxtReady`) and is now a deterministic unit test. Restructure pages to mirror the linkedin-insight fixtures: empty index, composable usage on `/calendly`. cdn fixture now overrides the page + registry config to disable bundling. Add unit test guarding the multi-arg push regression (#741) on the stub queue.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
test/unit/calendly-stub-queue.test.ts (1)
53-62: ⚡ Quick winMissing queue-length guard in the multi-arg test
The second test asserts the shape of
stub.q[0]but never verifies that exactly one entry was enqueued. If a future change to the stub accidentally pushes the entry multiple times (e.g., once per arg), the assertion onq[0]alone would still pass.🛡️ Proposed fix
it('preserves multiple positional args (showPopupWidget(url, ...))', () => { const stub = createStub() stub.showPopupWidget('https://calendly.com/example/30min', { foo: 'bar' }, 42) + expect(stub.q).toHaveLength(1) expect(stub.q[0]).toEqual([ 'showPopupWidget', 'https://calendly.com/example/30min', { foo: 'bar' }, 42, ]) })🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/unit/calendly-stub-queue.test.ts` around lines 53 - 62, The test for createStub's showPopupWidget should assert the queue length before inspecting stub.q[0]; add an explicit expectation that stub.q.length === 1 (or toEqual(1)) immediately after calling stub.showPopupWidget to ensure exactly one entry was enqueued, then keep the existing equality check on stub.q[0]; reference createStub, stub.showPopupWidget and stub.q when making this change.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test/fixtures/calendly/pages/calendly.vue`:
- Around line 11-14: The call to proxy.Calendly.initInlineWidget uses a CSS
selector string for parentElement but Calendly expects an actual DOM node;
change the invocation so parentElement is a resolved HTMLElement (e.g., obtain
the element via document.getElementById('calendly-host') or
document.querySelector('#calendly-host') and pass that variable into
proxy.Calendly.initInlineWidget) to ensure the widget mounts inside the intended
container.
---
Nitpick comments:
In `@test/unit/calendly-stub-queue.test.ts`:
- Around line 53-62: The test for createStub's showPopupWidget should assert the
queue length before inspecting stub.q[0]; add an explicit expectation that
stub.q.length === 1 (or toEqual(1)) immediately after calling
stub.showPopupWidget to ensure exactly one entry was enqueued, then keep the
existing equality check on stub.q[0]; reference createStub, stub.showPopupWidget
and stub.q when making this change.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 47210afd-2374-4e5b-b53e-cc841be0a607
📒 Files selected for processing (8)
package.jsontest/e2e/_calendly-suite.tstest/fixtures/calendly-cdn/nuxt.config.tstest/fixtures/calendly-cdn/pages/calendly.vuetest/fixtures/calendly/nuxt.config.tstest/fixtures/calendly/pages/calendly.vuetest/fixtures/calendly/pages/index.vuetest/unit/calendly-stub-queue.test.ts
✅ Files skipped from review due to trivial changes (3)
- package.json
- test/fixtures/calendly/nuxt.config.ts
- test/fixtures/calendly-cdn/pages/calendly.vue
🚧 Files skipped from review as they are similar to previous changes (3)
- test/fixtures/calendly/pages/index.vue
- test/fixtures/calendly-cdn/nuxt.config.ts
- test/e2e/_calendly-suite.ts
Calendly's `initInlineWidget` API requires a DOM element reference; CSS selector strings are not supported. Tighten the type and update fixtures to resolve the element via `querySelector` first. Per CodeRabbit review on #750.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🔗 Linked issue
Related to #177
❓ Type of change
📚 Description
Adds Calendly to the registry as
useScriptCalendly. Bundled and proxied viaassets.calendly.comwithPRIVACY_IP_ONLY; the booking iframe still hitscalendly.comdirectly since the vendor frame must load from origin. Composable auto-injects the widget stylesheet on first use, exposes a stub queue forinitInlineWidget/initPopupWidget/initBadgeWidget(full args spread to avoid the regression flagged on #741), and ships a typed surface for the popup/close helpers. Includes docs, playground, bundled + CDN fixtures, e2e suite, proxy/first-party unit rows, and the stub-queue regression guard.