-
Notifications
You must be signed in to change notification settings - Fork 9.3k
feat: Prerender booking link and reuse with headless router #20720
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
## What does this PR do? <!-- Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change. --> - Fixes #XXXX (GitHub issue number) - Fixes CAL-XXXX (Linear issue number - should be visible at the bottom of the GitHub issue description) ## Visual Demo (For contributors especially) A visual demonstration is strongly recommended, for both the original and new change **(video / image - any one)**. #### Video Demo (if applicable): - Show screen recordings of the issue or feature. - Demonstrate how to reproduce the issue, the behavior before and after the change. #### Image Demo (if applicable): - Add side-by-side screenshots of the original and updated change. - Highlight any significant change(s). ## Mandatory Tasks (DO NOT REMOVE) - [ ] I have self-reviewed the code (A decent size PR without self-review might be rejected). - [ ] I have updated the developer docs in /docs if this PR makes changes that would require a [documentation change](https://cal.com/docs). If N/A, write N/A here and check the checkbox. - [ ] I confirm automated tests are in place that prove my fix is effective or that my feature works. ## How should this be tested? <!-- Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration. Write details that help to start the tests --> - Are there environment variables that should be set? - What are the minimal test data to have? - What is expected (happy path) to have (input and output)? - Any other important info that could help to test that PR ## Checklist <!-- Remove bullet points below that don't apply to you --> - I haven't read the [contributing guide](https://github.com/calcom/cal.com/blob/main/CONTRIBUTING.md) - My code doesn't follow the style guidelines of this project - I haven't commented my code, particularly in hard-to-understand areas - I haven't checked if my changes generate no new warnings
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 2 Skipped Deployments
|
Hey there and thank you for opening this pull request! 👋🏼 We require pull request titles to follow the Conventional Commits specification and it looks like your proposed title needs to be adjusted. Details:
|
f2be740
to
5014dec
Compare
@@ -340,16 +416,37 @@ export const useEmbedType = () => { | |||
return state; | |||
}; | |||
|
|||
function unhideBody() { | |||
function makeBodyVisible() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Better named
@@ -36,13 +42,13 @@ customElements.define("cal-inline", Inline); | |||
|
|||
declare module "*.css"; | |||
type Namespace = string; | |||
type InitConfig = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Renamed InitConfig -> CalConfig as "init" is just how it is set. This was ambiguous with initialConfig
@@ -280,7 +290,23 @@ export class Cal { | |||
iframe.className = "cal-embed"; | |||
iframe.name = `cal-embed=${this.namespace}`; | |||
iframe.title = `Book a call`; | |||
const embedConfig = this.getInitConfig(); | |||
|
|||
this.loadInIframe({ calLink, config, calOrigin, iframe }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Abstracted it out from createIframe
as it now used separately as well.
9f8ff71
to
4b272ff
Compare
6b40bda
to
e621bdf
Compare
What does this PR do?
Fixes #20807
Video Demo (if applicable):
https://www.loom.com/share/448e7ee236684cf1842c0b9a914e1e99
PR Changes Summary
📝 Documentation:
+338 -1 (339 total)
🧪 Tests:
+1305 -14 (1319 total)
💻 Code:
+1278 -195 (1473 total)
Added router preloading to improve embed performance and reduce loading times. This feature preloads booking pages while routing form responses are being processed, creating a much faster user experience.
^Screenshot is from README
How to use
Just fire prerender instruction when you think is a good time. e.g. when user starts typing in the form(example in routing-playground.html)
Technical Details
/api/router
for headless router.prerender
instruction is used. This will change soon and will always be used later.Changes in App
getSchedule
orgetTeamSchedule
requests would have an additional parameterembedConnectVersion
(mapped from 'cal.embedConnectVersion' query param of the page)getSchedule
orgetTeamSchedule
requests will not be sent ifcal.skipSlotsFetch=true
param is set on the page.Documentation Improvements
See LIFECYCLE.md for full view
Tests
Mandatory Tasks (DO NOT REMOVE)
How should this be tested?
yarn seed-insights
. So that appropriate state of the DB is there to be able to test the scenario that is configured in routing-playgroundyarn dev
. It will open up embed playground which you could use to test other cases e.g. for regression testingFollowup