fix: Cal Video sidebar not showing meeting time in timezone #22821
Conversation
WalkthroughThe changes refactor the Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
🧰 Additional context used📓 Path-based instructions (3)**/*.ts📄 CodeRabbit Inference Engine (.cursor/rules/review.mdc)
Files:
**/*.{ts,tsx}📄 CodeRabbit Inference Engine (.cursor/rules/review.mdc)
Files:
**/*.tsx📄 CodeRabbit Inference Engine (.cursor/rules/review.mdc)
Files:
🧠 Learnings (4)📓 Common learningspackages/lib/dayjs/index.ts (1)Learnt from: CR apps/web/modules/videos/views/videos-single-view.tsx (1)Learnt from: CR apps/web/modules/bookings/views/bookings-single-view.tsx (3)Learnt from: CR Learnt from: eunjae-lee Learnt from: CR 🔇 Additional comments (6)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
| @@ -1229,8 +1229,21 @@ function RecurringBookings({ | |||
| <div key={idx} className={classNames("mb-2", isCancelled ? "line-through" : "")}> | |||
| {formatToLocalizedDate(dayjs.tz(dateStr, tz), language, "full", tz)} | |||
| <br /> | |||
| {formatToLocalizedTime(dayjs(dateStr), language, undefined, !is24h, tz)} -{" "} | |||
There was a problem hiding this comment.
If we're passing an object to formatToLocalizedTime we don't need to pass undefined as a param here.
There was a problem hiding this comment.
We are still setting timeStyle to undefined in the new object argument format, I think we should have skipped that.
| <h3>{t("when")}:</h3> | ||
| <p suppressHydrationWarning={true}> | ||
| {formatToLocalizedDate(startTime)} <br /> | ||
| {formatToLocalizedTime(startTime)} |
There was a problem hiding this comment.
Timezone wasn't being passed even though we're displaying it in the sidebar.
Graphite Automations"Add consumer team as reviewer" took an action on this PR • (07/30/25)1 reviewer was added to this PR based on Keith Williams's automation. |
E2E results are ready! |
| {formatToLocalizedTime({ | ||
| date: dayjs(dateStr), | ||
| locale: language, | ||
| hour12: !is24h, | ||
| timeZone: tz, | ||
| })}{" "} | ||
| -{" "} | ||
| {formatToLocalizedTime({ | ||
| date: dayjs(dateStr).add(duration, "m"), | ||
| locale: language, | ||
| hour12: !is24h, | ||
| timeZone: tz, | ||
| })}{" "} |
There was a problem hiding this comment.
nit: It might be better to have a fn that does returns this time range. It seems to be a duplicate here. Same as above.
| {formatToLocalizedTime({ date, locale: language, hour12: !is24h, timeZone: tz })} -{" "} | ||
| {formatToLocalizedTime({ | ||
| date: dayjs(date).add(duration, "m"), | ||
| locale: language, | ||
| hour12: !is24h, | ||
| timeZone: tz, | ||
| })}{" "} |
There was a problem hiding this comment.
This one too is duplicate it seems
| locale = undefined, | ||
| timeStyle = "short", | ||
| hour12 = undefined, | ||
| timeZone, |
There was a problem hiding this comment.
Would it make sense to make timeZone a required field, so that goign forward this timeZone isn't accidentally missed
What does this PR do?
formatToLocalizedTimeto accept an object instead of multiple paramsvideos-single-viewpass timezone toformatToLocalizedTimeMandatory Tasks (DO NOT REMOVE)
How should this be tested?