feat: Visual Indicator for AM/PM ( with respect to the phases of the day )#28801
feat: Visual Indicator for AM/PM ( with respect to the phases of the day )#28801vishalkumargeed wants to merge 2 commits intocalcom:mainfrom
Conversation
github-actions
bot
added
consumer
High priority
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe change extends 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/web/modules/bookings/components/AvailableTimes.tsx (1)
116-123: Consider computing slot display metadata once inAvailableTimesand passing it down.Right now timezone formatting runs in both parent (for width) and child (for label/hour). Precomputing
formattedTime(+slotHour) once per slot would reduce duplicate work and simplifySlotItem.Also applies to: 305-313
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/modules/bookings/components/AvailableTimes.tsx` around lines 116 - 123, Compute display metadata once inside AvailableTimes by creating shared values (e.g., computedDateWithUsersTimezone, formattedTime, and slotHour) for each slot and pass them into SlotItem as props; update the mapping where slots are rendered (the code using computedDateWithUsersTimezone.format(timeFormat) and any other timezone-based calculations in lines ~305-313) to use these precomputed values instead of recomputing in both the parent and SlotItem, and remove duplicate timezone/format logic from SlotItem so it solely uses the provided formattedTime and slotHour props.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/web/modules/bookings/components/AvailableTimes.tsx`:
- Around line 142-155: The color-selection block currently contains "what"
comments that restate behavior; remove the redundant comments around the
slotHour checks and assignments (the block using
computedDateWithUsersTimezone.hour() and bottomBarColor) and keep only brief
rationale comments if needed (e.g., explaining why specific color choices map to
time-of-day) so the code reads clearly without noise.
---
Nitpick comments:
In `@apps/web/modules/bookings/components/AvailableTimes.tsx`:
- Around line 116-123: Compute display metadata once inside AvailableTimes by
creating shared values (e.g., computedDateWithUsersTimezone, formattedTime, and
slotHour) for each slot and pass them into SlotItem as props; update the mapping
where slots are rendered (the code using
computedDateWithUsersTimezone.format(timeFormat) and any other timezone-based
calculations in lines ~305-313) to use these precomputed values instead of
recomputing in both the parent and SlotItem, and remove duplicate
timezone/format logic from SlotItem so it solely uses the provided formattedTime
and slotHour props.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d492c081-3935-4ed3-bcd0-1e318e00c3d8
📒 Files selected for processing (1)
apps/web/modules/bookings/components/AvailableTimes.tsx
With the slots or additional information shown to the user with the colors they can somewhat subconciously identify the slots
|
1 participant
What does this PR do?
Here is the detailed walkthrough this PR
Note : I checked the open PR and with resepect to that I guess this is most visual indicator and possibly the correct UI enhancement , as I'm not adding any kind of icon , just adding a small color div at the bottom of the button component
computedDateWithUsersTimezone.format(timeFormat)Result
Without “Overlay my calender”
With “Overlay my calender”
Other Possibilities
1. Placing the color bar just at the bottom of the button component and has a fixed size of
h-[2px]this looks even more good but but but<div className="pointer-events-none absolute right-0 bottom-0 left-0 flex justify-center" aria-hidden="true"> <span className={classNames("block h-[2px] rounded-t-full", bottomBarColor)} style={{ width:${Math.max(bottomBarWidthCh ?? 0, formattedTime.length)}ch}} /> </div>Uneven height of the color component . Pattern : the even sequence button (2,4,6..) height decrease and for odd sequence of button the height is maintained
Similarly goes with when switched to column view
Uneven height of the color component . Pattern : the even sequence button (2,4,6..) height decrease and for odd sequence of button the height is maintained
For this the height is reduced for odd number of sequence button (if you carefully observe ! )
2. Uses more good approach by placing the color bar just above the bottom of the button and increase the bar thickness (height)
h-1is the ideal height of the color bar as if we reduce the height toh-0.5it will act same as the Possibility-1bottom-0notbottom-pxaspxwill create a spacing of1pxwhich looks somewhat appered (which In my opinion doesn’t align with the UI theme )Mandatory Tasks (DO NOT REMOVE)
How should this be tested?
Manual
Checklist