Conversation
Fixed all the errors, logical issues, UI/UX improved
|
@harshitap1305 is attempting to deploy a commit to the openlake's projects Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughThe pull request restructures the sidebar layout with improved navigation scrolling, updates a navigation label for consistency, and refactors how the RoomBookingPage component passes configuration data to the Layout component through new props. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~15 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 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 |
There was a problem hiding this comment.
Pull request overview
This PR updates the Room Booking frontend to use the app’s grid-based Layout API and significantly redesigns the Room Booking UI with a timeline-based booking workflow, modal-driven forms, and richer admin/user views.
Changes:
- Refactors
RoomBookingPageto renderRoomBookingviaLayout’sgridConfig+componentspattern. - Reworks
RoomBookingUI: drag-to-select timeline, filtering (capacity/amenities), modal flows for booking/room creation/rejection, and improved approvals/history views. - Adjusts sidebar nav rendering to be scrollable and updates navbar labels for the room booking entry.
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 9 comments.
| File | Description |
|---|---|
frontend/src/pages/roomBookingPage.jsx |
Switches page rendering to Layout grid configuration. |
frontend/src/config/navbarConfig.js |
Updates Room Booking nav label text. |
frontend/src/Components/RoomBooking.jsx |
Major UI/logic overhaul: timeline selection, modals, filters, approvals/history. |
frontend/src/Components/common/Sidebar.jsx |
Makes nav list scrollable and repositions logout section for better layout behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const xToHour = (clientX) => { | ||
| if (!trackRef.current) return HOUR_START; | ||
| const rect = trackRef.current.getBoundingClientRect(); | ||
| const x = clientX - rect.left + trackRef.current.scrollLeft; |
There was a problem hiding this comment.
xToHour() adds trackRef.current.scrollLeft, but trackRef is attached to the inner timeline div while the horizontal scrolling happens on the outer overflow-x-auto container. When the user scrolls horizontally, the hour calculation will be offset and selected slots/bars won’t line up. Attach the ref to the actual scroll container (or read scrollLeft from the wrapper via a separate ref/parentElement) so pointer-to-hour math stays correct while scrolled.
| const x = clientX - rect.left + trackRef.current.scrollLeft; | |
| const scrollLeft = trackRef.current.parentElement?.scrollLeft ?? 0; | |
| const x = clientX - rect.left + scrollLeft; |
| const pad = (n) => String(Math.floor(n)).padStart(2, "0"); | ||
| const minStr = (h) => (h % 1 === 0.5 ? "30" : "00"); | ||
| const startStr = `${pad(s)}:${minStr(s)}`; | ||
| const endStr = `${pad(e)}:${minStr(e)}`; | ||
| onSlotSelect(startStr, endStr); |
There was a problem hiding this comment.
The drag selection can produce an endStr of 24:00 (because xToHour is clamped to HOUR_END and then formatted with pad()), but 24:00 is not a valid value for <input type="time"> and will also create an invalid Date in combineDateTime. Clamp the selectable range to the last valid slot (e.g., 23:30 for 30-min increments) or explicitly handle 24:00 by rolling over to the next day.
| id="filter-room" | ||
| value={filters.roomId} | ||
| onChange={(e) => setFilters((p) => ({ ...p, roomId: e.target.value }))} | ||
| className="w-full rounded-md border border-slate-300 px-3 py-1.5 text-sm focus:outline-none focus:ring-2 focus:ring-slate-500/20 focus:border-slate-500 transition-all bg-white" | ||
| > | ||
| {amenity} | ||
| <button type="button" onClick={() => removeAmenity(amenity)}> | ||
| <X className="h-3.5 w-3.5" /> | ||
| {filteredRooms.map((room) => ( | ||
| <option key={room._id} value={room._id}> | ||
| {room.name} ({room.capacity} seats) |
There was a problem hiding this comment.
The room selector options are derived from filteredRooms, but filters.roomId is not normalized when capacity/amenity filters change. If the currently-selected roomId is no longer present in filteredRooms, the <select> will have a value that doesn’t match any option, leading to a blank selection and inconsistent timeline/results. Consider resetting filters.roomId to the first matching filteredRooms[0]._id (or empty) whenever the filtered list changes.
| return ( | ||
| <div | ||
| className="fixed inset-0 z-50 flex items-center justify-center p-4 sm:p-6" | ||
| style={{ background: "rgba(15,23,42,0.6)", backdropFilter: "blur(6px)" }} | ||
| onMouseDown={(e) => e.target === e.currentTarget && onClose()} | ||
| > | ||
| <div | ||
| className="bg-white rounded-lg shadow-xl w-full max-w-lg max-h-[90vh] flex flex-col" | ||
| style={{ animation: "modalIn 0.25s cubic-bezier(0.16, 1, 0.3, 1)" }} |
There was a problem hiding this comment.
The custom modal wrapper renders a dialog visually, but it doesn’t expose dialog semantics to assistive tech (e.g., missing role="dialog", aria-modal="true", and an aria-labelledby/aria-label). Add appropriate ARIA attributes (and ideally focus management) so screen readers and keyboard users can reliably interact with the modal.
| { key: "announcements", label: "Announcements", icon: Megaphone }, | ||
| { key: "events", label: "Events", icon: Calendar }, | ||
| { key: "roombooking", label: "Room Booking", icon: DoorOpen }, | ||
| { key: "roombooking", label: "RoomBooking", icon: DoorOpen }, | ||
| { key: "endorsement", label: "Endorsements", icon: Award }, |
There was a problem hiding this comment.
Navbar label changed to RoomBooking (no space). This will render as a single word in the sidebar/navbar and is inconsistent with other human-readable labels. If this wasn’t intentional, change it back to Room Booking.
| { key: "por", label: "PORs", icon: UserPlus }, | ||
| { key: "feedback", label: "Feedback", icon: ClipboardList }, | ||
| { key: "events", label: "Events", icon: Calendar }, | ||
| { key: "roombooking", label: "Room Booking", icon: DoorOpen }, | ||
| { key: "roombooking", label: "RoomBooking", icon: DoorOpen }, | ||
| // { key: "add-event", label: "Add Event", icon: Plus }, |
There was a problem hiding this comment.
Navbar label changed to RoomBooking (no space). This will render as a single word in the sidebar/navbar and is inconsistent with other human-readable labels. If this wasn’t intentional, change it back to Room Booking.
|
|
||
| { key: "por", label: "PORs", icon: UserPlus }, | ||
| { key: "events", label: "Events", icon: Calendar }, | ||
| { key: "roombooking", label: "Room Booking", icon: DoorOpen }, | ||
| { key: "roombooking", label: "RoomBooking", icon: DoorOpen }, | ||
| // { key: "add-event", label: "Add Event", icon: Plus }, |
There was a problem hiding this comment.
Navbar label changed to RoomBooking (no space). This will render as a single word in the sidebar/navbar and is inconsistent with other human-readable labels. If this wasn’t intentional, change it back to Room Booking.
| const RejectionModal = ({ open, onClose, onConfirm }) => { | ||
| const [reason, setReason] = useState(""); | ||
| const handleConfirm = () => { | ||
| onConfirm(reason.trim()); | ||
| setReason(""); | ||
| }; |
There was a problem hiding this comment.
RejectionModal only clears the reason state on confirm. If the modal is closed via Cancel/Escape/click-outside, the previous reason will still be present the next time it opens. Clear reason when open becomes true/false or inside onClose to avoid carrying stale input across requests.
| }, [open, initialData, rooms]); | ||
|
|
There was a problem hiding this comment.
In BookingModal, the reset/load effect depends on the full rooms array. If rooms changes while the modal is open (e.g., user hits Refresh in the background), this effect will re-run and overwrite in-progress form edits. Consider only resetting on open/initialData changes, or guard the rooms dependency so it only updates roomId when it’s empty/invalid.
| }, [open, initialData, rooms]); | |
| }, [open, initialData]); | |
| useEffect(() => { | |
| if (!open) return; | |
| setForm((prev) => { | |
| const hasValidRoom = prev.roomId && rooms.some((room) => room._id === prev.roomId); | |
| if (hasValidRoom) return prev; | |
| const nextRoomId = rooms[0]?._id || ""; | |
| if (prev.roomId === nextRoomId) return prev; | |
| return { | |
| ...prev, | |
| roomId: nextRoomId, | |
| }; | |
| }); | |
| }, [open, rooms]); |
![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)
frontend/src/Components/common/Sidebar.jsx (1)
55-57: Useitem.keyinstead of label text for behavior checks.Line 57 couples logic to display text (
"Profile"). This is brittle after label edits and will break with future copy/i18n changes. Use the stable key ("profile") instead.♻️ Suggested change
- item.label !== "Profile" && ( + item.key !== "profile" && (🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/Components/common/Sidebar.jsx` around lines 55 - 57, The map callback in Sidebar.jsx currently checks item.label !== "Profile", which couples behavior to display text; update the condition to use the stable identifier item.key (e.g., item.key !== "profile") inside the navItems.map callback so the display logic relies on the stable key rather than the label; modify any related checks in the same callback to reference item.key instead of item.label.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/src/pages/roomBookingPage.jsx`:
- Around line 15-17: The layout for the RoomBooking panel hardcodes colEnd: 20
which is wrong when the sidebar is collapsed and GridLayout uses 25 columns;
update the layout creation (the object passed to GridLayout / the RoomBooking
panel layout in roomBookingPage.jsx) to compute colEnd dynamically based on the
current grid column count (use the sidebar collapse state or the same variable
GridLayout uses, e.g. sidebarCollapsed or gridCols) — for example set const
gridCols = sidebarCollapsed ? 25 : 20 and use colEnd: gridCols (or compute
colEnd = Math.min(desiredWidth, gridCols)) so the panel spans the full width in
collapsed mode. Ensure you update any place that constructs the layout object
for RoomBooking (the colStart/colEnd entry) to use this computed value.
---
Nitpick comments:
In `@frontend/src/Components/common/Sidebar.jsx`:
- Around line 55-57: The map callback in Sidebar.jsx currently checks item.label
!== "Profile", which couples behavior to display text; update the condition to
use the stable identifier item.key (e.g., item.key !== "profile") inside the
navItems.map callback so the display logic relies on the stable key rather than
the label; modify any related checks in the same callback to reference item.key
instead of item.label.
🪄 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: 7c61751d-9dcf-44bb-845b-62800e014b43
📒 Files selected for processing (4)
frontend/src/Components/RoomBooking.jsxfrontend/src/Components/common/Sidebar.jsxfrontend/src/config/navbarConfig.jsfrontend/src/pages/roomBookingPage.jsx
| colStart: 0, | ||
| colEnd: 20, | ||
| rowStart: 0, |
There was a problem hiding this comment.
Grid width is truncated when sidebar is collapsed.
Line 16 hardcodes colEnd: 20, but GridLayout switches to 25 columns when collapsed. This leaves unused columns and shrinks the RoomBooking panel in collapsed mode.
🐛 Suggested fix
import React from "react";
import Layout from "../Components/common/Layout";
import RoomBooking from "../Components/RoomBooking";
+import { useSidebar } from "../hooks/useSidebar";
const RoomBookingPage = () => {
+ const { isCollapsed } = useSidebar();
const components = {
RoomBooking: RoomBooking,
};
const gridConfig = [
{
id: "main",
component: "RoomBooking",
position: {
colStart: 0,
- colEnd: 20,
+ colEnd: isCollapsed ? 25 : 20,
rowStart: 0,
rowEnd: 16,
},
},
];📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| colStart: 0, | |
| colEnd: 20, | |
| rowStart: 0, | |
| import React from "react"; | |
| import Layout from "../Components/common/Layout"; | |
| import RoomBooking from "../Components/RoomBooking"; | |
| import { useSidebar } from "../hooks/useSidebar"; | |
| const RoomBookingPage = () => { | |
| const { isCollapsed } = useSidebar(); | |
| const components = { | |
| RoomBooking: RoomBooking, | |
| }; | |
| const gridConfig = [ | |
| { | |
| id: "main", | |
| component: "RoomBooking", | |
| position: { | |
| colStart: 0, | |
| colEnd: isCollapsed ? 25 : 20, | |
| rowStart: 0, | |
| rowEnd: 16, | |
| }, | |
| }, | |
| ]; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/pages/roomBookingPage.jsx` around lines 15 - 17, The layout for
the RoomBooking panel hardcodes colEnd: 20 which is wrong when the sidebar is
collapsed and GridLayout uses 25 columns; update the layout creation (the object
passed to GridLayout / the RoomBooking panel layout in roomBookingPage.jsx) to
compute colEnd dynamically based on the current grid column count (use the
sidebar collapse state or the same variable GridLayout uses, e.g.
sidebarCollapsed or gridCols) — for example set const gridCols =
sidebarCollapsed ? 25 : 20 and use colEnd: gridCols (or compute colEnd =
Math.min(desiredWidth, gridCols)) so the panel spans the full width in collapsed
mode. Ensure you update any place that constructs the layout object for
RoomBooking (the colStart/colEnd entry) to use this computed value.
Pull request overview
This PR updates the Room Booking frontend to use the app’s grid-based
LayoutAPI and significantly redesigns the Room Booking UI with a timeline-based booking workflow, modal-driven forms, and richer admin/user views.Changes:
RoomBookingPageto renderRoomBookingviaLayout’sgridConfig+componentspattern.RoomBookingUI: drag-to-select timeline, filtering (capacity/amenities), modal flows for booking/room creation/rejection, and improved approvals/history views.Reviewed changes
frontend/src/pages/roomBookingPage.jsxLayoutgrid configuration.frontend/src/config/navbarConfig.jsfrontend/src/Components/RoomBooking.jsxfrontend/src/Components/common/Sidebar.jsxSummary by CodeRabbit
UI Improvements
Style