fix: browser back button not working

[abhijeetsingh-22]

What does this PR do?

Fixes a bug due to which the back button on the booking page was not working. The issue was due to multiple calls to setQueryParams and removeQueryParams as it was pushing multiple URLs to the browser history.

My fix

Instead of pushing to the history I am now replacing the last entry in the history.

Fixes #13324

https://github.com/calcom/cal.com/assets/55000107/79f9645e-2264-4f47-bb85-d00c81fec69d

Type of change

• Bug fix (non-breaking change which fixes an issue)

How should this be tested?

• Go to the booking page of a user with multiple events present.
• Open one of the events and change the date selected.
• Press the browser back button (You should be taken back to the user page.)
• Try repeating this with the other event .

Mandatory Tasks

• [ x] Make sure you have self-reviewed the code. A decent size PR without self-review might be rejected.

[vercel[bot]]

@abhijeetsingh-22 is attempting to deploy a commit to the cal Team on Vercel.

A member of the Team first needs to authorize it.

[github-actions[bot]]

Thank you for following the naming conventions! 🙏 Feel free to join our discord and post your PR link.

[github-actions[bot]]

📦 Next.js Bundle Analysis for @calcom/web

This analysis was generated by the Next.js Bundle Analysis action. 🤖

This PR introduced no changes to the JavaScript bundle! 🙌

[vercel[bot]]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
dev	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Jan 23, 2024 7:56am

1 Ignored Deployment

Name	Status	Preview	Comments	Updated (UTC)
calcom-web-canary	⬜️ Ignored (Inspect)	Visit Preview		Jan 23, 2024 7:56am

[emrysal]

@abhijeetsingh-22 looks like Udit is right; not an issue with your PR but it looks like the history is not set properly (perhaps due to next router) when coming from the booking page?

[abhijeetsingh-22]

@abhijeetsingh-22 looks like Udit is right; not an issue with your PR but it looks like the history is not set properly (perhaps due to next router) when coming from the booking page?

@emrysal The problem is not the next-router but the way we are handling the queryParams in the /[user]/[type] . I have made the changes have a look

https://github.com/calcom/cal.com/assets/55000107/ab09f63e-511c-41dd-ad59-277c7b3025fa

[emrysal]

@abhijeetsingh-22 before approving just want to run something by you - do you think it makes sense to move the "is it a push/replace" to another parameter in the removeQueryParam function? Just because the literal check for "slot" feels like too much of an unexpected side-effect to me.

[abhijeetsingh-22]

@abhijeetsingh-22 before approving just want to run something by you - do you think it makes sense to move the "is it a push/replace" to another parameter in the removeQueryParam function? Just because the literal check for "slot" feels like too much of an unexpected side-effect to me.

I think it makes sense to move isReplace to function parameter. It will make the function more reusable. Thanks for the suggestion I will make the changes and push shortly

[PeerRich]

/bonus 20

hey @abhijeetsingh-22 any chance you can take a look at the merge conflict and the changes mentioned above?

[algora-pbc[bot]]

A bonus of $20 has been added by PeerRich. @abhijeetsingh-22: You will receive $20 once you implement the requested changes and your PR is merged.

[abhijeetsingh-22]

/bonus 20

hey @abhijeetsingh-22 any chance you can take a look at the merge conflict and the changes mentioned above?

Got a little busy. Will do it today

[abhijeetsingh-22]

@emrysal made the changes pl review again.

[devklepacki]

Hello guys. I've stumbled across this issue and wanted to add something that may be potentially useful or maybe without any importance. In Next's Router (from next/router) object and in the Link component (next/link) there's an option to perform shallow routing. that I've always been using for such cases. It works exactly as we'd like it to, replacing url without altering the browser's history state.

It's just nicer to write:

router.replace(Url.toString(), undefined, { shallow: isShallow })

Than to

if (shouldReplace) {
    window.history.replaceState({ ...window.history.state, as: url.href }, "", url.href);
} else {
    window.history.pushState({ ...window.history.state, as: url.href }, "", url.href);
}

(that btw seems to be duplicated in this PR 😅 once in updateQueryParam and secondly in removeQueryParam instead of being pushed outside as an util function)

Anyway, I'm not sure if this brings any value to the issue. Can't wait for this major bug fix to happen 🙌

[abhijeetsingh-22]

Hello guys. I've stumbled across this issue and wanted to add something that may be potentially useful or maybe without any importance. In Next's Router (from next/router) object and in the Link component (next/link) there's an option to perform shallow routing. that I've always been using for such cases. It works exactly as we'd like it to, replacing url without altering the browser's history state.

It's just nicer to write:

router.replace(Url.toString(), undefined, { shallow: isShallow })

Than to

if (shouldReplace) {
    window.history.replaceState({ ...window.history.state, as: url.href }, "", url.href);
} else {
    window.history.pushState({ ...window.history.state, as: url.href }, "", url.href);
}

(that btw seems to be duplicated in this PR 😅 once in updateQueryParam and secondly in removeQueryParam instead of being pushed outside as an util function)

Anyway, I'm not sure if this brings any value to the issue. Can't wait for this major bug fix to happen 🙌

Thanks for the suggestion. But here are few things to consider

• The router comes from useRouter so if we want to use it we need to convert the query-param util to a hook.
• Even if we use router we have to have an additional param that will tell if we should use router.replace or router.push as for setting slot we should be using push because we need to have a browser history entry for it and for other query-params we should be using replace.

[PeerRich]

checks are failing @abhijeetsingh-22

[abhijeetsingh-22]

I will look into it

[algora-pbc[bot]]

🎉🎈 @abhijeetsingh-22 has been awarded $20! 🎈🎊

• Share on socials
• Give feedback