-
Notifications
You must be signed in to change notification settings - Fork 11.5k
feat: integrate BookingHistory with Bookings V3 design #26301
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
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
|
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: |
- Updated the booking page to retrieve user feature statuses for "bookings-v3" and "booking-audit". - Modified BookingDetailsSheet and BookingListContainer components to accept and utilize the new bookingAuditEnabled prop. - Adjusted the BookingHistory display logic to conditionally render based on the bookingAuditEnabled state. - Refactored SegmentedControl to support generic types for better type safety. This change improves the user experience by allowing conditional access to booking audit features based on user permissions.
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.
1 issue found across 14 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="packages/ui/components/segmented-control/SegmentedControl.tsx">
<violation number="1" location="packages/ui/components/segmented-control/SegmentedControl.tsx:58">
P1: Hardcoded radio button `name` will cause conflicts when multiple SegmentedControl instances exist on the same page. Consider using React's `useId()` hook to generate a unique name per component instance.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
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.
1 issue found across 4 files (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="apps/web/modules/bookings/components/BookingDetailsSheet.tsx">
<violation number="1" location="apps/web/modules/bookings/components/BookingDetailsSheet.tsx:102">
P1: When `activeSegment` is `null` (initial state or after closing sheet), neither the info nor history content will render because strict equality checks fail. The function should default to `"info"` when `activeSegment` is `null` to match the original behavior.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
…and store - Removed local state management for active segment in BookingDetailsSheet and integrated Zustand store for better state synchronization. - Introduced useActiveSegmentFromUrl hook to sync active segment with URL query parameters. - Updated BookingDetailsSheet to handle derived active segment logic based on bookingAuditEnabled state. - Adjusted SegmentedControl to ensure it defaults to "info" when activeSegment is null. This refactor enhances the maintainability and responsiveness of the booking details component.
3f38cd7 to
6e38e44
Compare
| if (!bookingAuditEnabled && activeSegment === "history") { | ||
| return "info"; | ||
| } | ||
| return activeSegment ?? "info"; |
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.
Ensures that if a state is invalid as per feature flag, it is not transitioned to.
| useEffect(() => { | ||
| const unsubscribe = store.subscribe((state) => { | ||
| const storeUid = state.selectedBookingUid; | ||
| if (storeUid !== selectedBookingUidFromUrl) { | ||
| setSelectedBookingUidToUrl(storeUid); | ||
| } | ||
| }); | ||
|
|
||
| return unsubscribe; | ||
| }, [selectedBookingUidFromUrl, setSelectedBookingUidToUrl, store]); | ||
|
|
||
| // Sync URL → Store | ||
| useEffect(() => { | ||
| const currentStoreUid = store.getState().selectedBookingUid; | ||
| if (currentStoreUid !== selectedBookingUidFromUrl) { | ||
| store.getState().setSelectedBookingUid(selectedBookingUidFromUrl); | ||
| } | ||
| }, [selectedBookingUidFromUrl, store]); |
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.
Moved to separate hook useBiDirectionalSyncBetweenZustandAndNuqs and fixed a race condition.
Also, added a TODO to avoid useEffect syncing entirely.
| type="radio" | ||
| id={inputId} | ||
| value={itemValue} | ||
| checked={isActive} | ||
| onChange={() => handleChange(itemValue)} | ||
| disabled={disabled} | ||
| className="sr-only" | ||
| aria-checked={isActive} |
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.
why are you using not using our components from @calcom/ui?
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.
I did not see a component for this purpose in there. In Figma it was named SegementedControl, so I am using that only.
Could you confirm if it or similar one exists there?
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.
There is a Radio component in packages/ui/components/radio/Radio.tsx. We can create more variant if we want the UI to be different
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.
I think SegementedControl makes more sense. radix ui also has this https://www.radix-ui.com/themes/docs/components/segmented-control
volnei
left a comment
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.
small comment, happy to approve when fixed 💯
…rectionalSyncBetweenStoreAndUrl Co-Authored-By: hariom@cal.com <hariombalhara@gmail.com>
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.
1 issue found across 16 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="apps/web/modules/bookings/views/bookings-view.tsx">
<violation number="1" location="apps/web/modules/bookings/views/bookings-view.tsx:84">
P2: `bookingAuditEnabled` is passed to `BookingListContainer` but not to `BookingCalendarContainer`. Users viewing bookings from the calendar view won't see the Info/History segmented control even when the `booking-audit` feature flag is enabled. For consistency, `BookingCalendarContainer` should also receive and forward this prop to `BookingDetailsSheet`.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
* Integrate BookingHistory with Bookings V3 design * Enhance booking features by integrating booking audit functionality - Updated the booking page to retrieve user feature statuses for "bookings-v3" and "booking-audit". - Modified BookingDetailsSheet and BookingListContainer components to accept and utilize the new bookingAuditEnabled prop. - Adjusted the BookingHistory display logic to conditionally render based on the bookingAuditEnabled state. - Refactored SegmentedControl to support generic types for better type safety. This change improves the user experience by allowing conditional access to booking audit features based on user permissions. * Refactor BookingDetailsSheet to manage active segment state with Zustand store - Removed local state management for active segment in BookingDetailsSheet and integrated Zustand store for better state synchronization. - Introduced useActiveSegmentFromUrl hook to sync active segment with URL query parameters. - Updated BookingDetailsSheet to handle derived active segment logic based on bookingAuditEnabled state. - Adjusted SegmentedControl to ensure it defaults to "info" when activeSegment is null. This refactor enhances the maintainability and responsiveness of the booking details component. * refactor: rename useBiDirectionalSyncBetweenZustandAndNuqs to useBiDirectionalSyncBetweenStoreAndUrl Co-Authored-By: hariom@cal.com <hariombalhara@gmail.com> --------- Co-authored-by: Udit Takkar <53316345+Udit-takkar@users.noreply.github.com> Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
What does this PR do?
Integrates the Booking History feature into the Bookings V3 design by adding an Info/History segmented control in the booking details sheet. This allows users to view booking audit logs directly within the booking details panel instead of navigating to a separate page.
Loom Demo
Key changes:
SegmentedControlUI component to@calcom/uiBookingDetailsSheet(gated bybooking-auditfeature flag)BookingLogsViewto@calcom/features/booking-auditasBookingHistoryFeaturesRepository.getUserFeaturesStatus()method for efficient batch feature flag checksBookingHistoryPagewrapper for standalone page contextUpdates since last revision
useBiDirectionalSyncBetweenZustandAndNuqstouseBiDirectionalSyncBetweenStoreAndUrlto avoid using provider names in concrete implementationsMandatory Tasks (DO NOT REMOVE)
How should this be tested?
booking-auditfeature flag for a test user/booking/[uid]/logspage still works with the newBookingHistoryPagecomponentWithout feature flag:
booking-auditflag is disabledHuman Review Checklist
SegmentedControlcomponent renders correctly and is accessiblebooking-auditdisabled)getUserFeaturesStatushandles edge cases (empty slugs array, missing features)