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
fix: browser back button not working #13345
fix: browser back button not working #13345
Conversation
@abhijeetsingh-22 is attempting to deploy a commit to the cal Team on Vercel. A member of the Team first needs to authorize it. |
Thank you for following the naming conventions! 🙏 Feel free to join our discord and post your PR link. |
📦 Next.js Bundle Analysis for @calcom/webThis analysis was generated by the Next.js Bundle Analysis action. 🤖 This PR introduced no changes to the JavaScript bundle! 🙌 |
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Ignored Deployment
|
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.
Screen.Recording.2024-01-22.at.2.31.28.PM.mov
I should be back to the booking form page and not /[user]
@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 Kapture.2024-01-23.at.04.26.14.mp4 |
@@ -213,6 +213,10 @@ const BookerComponent = ({ | |||
return setBookerState("booking"); | |||
}, [event, selectedDate, selectedTimeslot, setBookerState]); | |||
|
|||
const slot = getQueryParam("slot"); | |||
useEffect(() => { | |||
setSelectedTimeslot(slot || null); |
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.
This makes sure that the selectedTimeslot
updates in the store when it is changed in the query param. And it sets it to null when the slot
query param is not present
} else { | ||
url.searchParams.set(param, `${value}`); | ||
} | ||
|
||
window.history.pushState({}, "", url.href); | ||
if (param == "slot") { |
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.
For the desired behavior, we need to smartly choose between pushState
and replaceState
so that the browser history is created the way we want. This is in-line with the change that @Udit-takkar requested earlier.
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.
The changes cause booking-limits e2e tests to fail.
I found one issue that happens in https://github.com/calcom/cal.com/blob/main/apps/web/playwright/booking-limits.e2e.ts#L306. For monthly duration limits, it doesn't redirect to the booking page but stays on the availability page instead
I will look into it |
🎉🎈 @abhijeetsingh-22 has been awarded $20! 🎈🎊 |
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
andremoveQueryParams
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
Calcom.Back.button.fix.mp4
Type of change
How should this be tested?
Mandatory Tasks