Uh oh!
There was an error while loading. Please reload this page.
chore(calendly): address CodeRabbit follow-ups from #750 - #758
Conversation
Three issues raised after the squash-merge of the Calendly PR:
1. **(major) Stylesheet disappears after SPA navigation.** The
`cssInjected` module-level flag short-circuited
`ensureCalendlyStylesheet()` after first injection. But because the
`useHead` call runs inside the composable's `setup()`, the entry is
component-scoped — unhead removes the `<style>` on unmount. On the
next visit the flag still says "already injected", so the widget
loaded unstyled. Drop the flag and rely on `key` deduplication.
2. **(minor) Watch on `status` missed pre-existing error state.** A
shared `useScriptCalendly()` instance that already failed (e.g. on a
prior navigation) won't re-fire `status` change on a new component
mount. Add `{ immediate: true }` so the component emits `error`
when it encounters an already-failed instance.
3. **(minor) `CalendlyPageSettings` missing `hideGdprBanner`.**
Calendly's SDK accepts it; the component's local copy of the
interface had it but the registry's exported one didn't, so callers
composing `CalendlyInlineWidgetOptions` / `CalendlyPopupWidgetOptions`
directly couldn't pass the flag type-safely.The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThis pull request makes two focused improvements to the Calendly widget integration. First, the status watcher in Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 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 |
commit: |
Uh oh!
There was an error while loading. Please reload this page.
🔗 Linked issue
Follow-up to #750.
❓ Type of change
📚 Description
Three issues raised by CodeRabbit on #750 that landed after the squash-merge:
🟠 Major: stylesheet disappears after SPA navigation
The
cssInjectedmodule-level flag short-circuitedensureCalendlyStylesheet()after first injection. But because theuseHeadcall runs inside the composable'ssetup(), the entry is component-scoped — unhead removes the<style>on unmount. On the next visit the flag still said "already injected", so the widget loaded unstyled.Fix: drop the flag and rely on
keydeduplication. Multiple components on the same page callinguseHeadwith the samekeyare safely collapsed to one<style>tag, and the entry follows the component lifecycle correctly across SPA navigations.🟡 Minor:
watchonstatusmissed pre-existing'error'stateA shared
useScriptCalendly()instance that already failed (e.g. on a prior navigation) won't re-firestatuschange on a new component mount. The component never emitserrorfor the existing failure. Added{ immediate: true }.🟡 Minor:
CalendlyPageSettingsmissinghideGdprBannerCalendly's SDK accepts it; the component's local copy of the interface had it but the registry's exported one didn't, so callers composing
CalendlyInlineWidgetOptions/CalendlyPopupWidgetOptionsdirectly couldn't pass the flag type-safely.✅ Verification
/third-parties/calendly/nuxt-scripts, navigate back →<style>was missing); fixed after.