[BOOKINGSG-6090-timetable][JZ] Apply PR comments

LifeSG · Oct 3, 2024 · e3ad33a · e3ad33a
1 parent 162f245
commit e3ad33a
Show file tree

Hide file tree

Showing 15 changed files with 4,662 additions and 366 deletions.
diff --git a/src/popover-v2/popover-trigger.tsx b/src/popover-v2/popover-trigger.tsx
@@ -27,6 +27,8 @@ export const PopoverTrigger = ({
     position = "top",
     zIndex,
     rootNode,
+    customOffset,
+    delay,
     onPopoverAppear,
     onPopoverDismiss,
     ...otherProps
@@ -45,7 +47,7 @@ export const PopoverTrigger = ({
         placement: position,
         whileElementsMounted: autoUpdate,
         middleware: [
-            offset(otherProps.offset ?? 16),
+            offset(customOffset ?? 16),
             flip(),
             shift({
                 limiter: limitShift(),
@@ -72,8 +74,8 @@ export const PopoverTrigger = ({
         enabled: trigger === "hover",
         // short window to enter the floating element without it closing
         delay: {
-            open: otherProps?.delay?.open ?? 0,
-            close: otherProps?.delay?.close ?? 500,
+            open: delay?.open ?? 0,
+            close: delay?.close ?? 500,
         },
     });
 

diff --git a/src/popover-v2/types.ts b/src/popover-v2/types.ts
@@ -33,9 +33,11 @@ export interface PopoverV2TriggerProps {
      * the popover may not be visible. Specify the parent element here instead
      */
     rootNode?: RefObject<HTMLElement> | undefined;
-    offset?: number | undefined;
+    customOffset?: number | undefined;
     // in milliseconds
-    delay?: { open: number; close: number } | undefined;
+    delay?:
+        | { open?: number | undefined; close?: number | undefined }
+        | undefined;
     onPopoverAppear?: (() => void) | undefined;
     onPopoverDismiss?: (() => void) | undefined;
 }

diff --git a/src/timetable/const.ts b/src/timetable/const.ts
@@ -0,0 +1,10 @@
+export const ROW_BAR_COLOR_SEQUENCE = [
+    "#FFE6BB",
+    "#D8EFEB",
+    "#E6EAFE",
+    "#FAE4E5",
+    "#D3EEFC",
+] as const; // Assert to be a readonly tuple
+export const ROW_CELL_GAP = 2;
+export const ROW_INTERVAL = 15;
+export type RowBarColors = (typeof ROW_BAR_COLOR_SEQUENCE)[number];
diff --git a/src/timetable/timetable-row/row-bar.tsx b/src/timetable/timetable-row/row-bar.tsx
@@ -1,23 +1,21 @@
 import dayjs from "dayjs";
 import { MutableRefObject } from "react";
+import { RowBarColors } from "../const";
 import {
     CustomPopoverProps,
-    RowBarColors,
-    RowCellData,
-    RowData,
+    TimeTableRowCellData,
+    TimeTableRowData,
 } from "../types";
 import { RowCellContainer } from "./row-bar.style";
 import { RowCell } from "./row-cell";
 
-interface RowBarProps extends RowData {
+interface RowBarProps extends TimeTableRowData {
     timetableMinTime: string;
     timetableMaxTime: string;
     intervalWidth: number;
     containerRef: MutableRefObject<HTMLDivElement>;
     rowBarColor: RowBarColors;
     outsideOpHoursCellCustomPopover?: CustomPopoverProps | undefined;
-    "data-testid"?: string | undefined;
-    onCellClick?: (data: RowCellData, e: React.MouseEvent) => void;
 }
 
 export const RowBar = ({
@@ -30,12 +28,14 @@ export const RowBar = ({
     rowBarColor,
     intervalWidth,
     containerRef,
-    ...optionalProps
+    outsideOpHoursCellCustomPopover,
+    ...otherProps
 }: RowBarProps) => {
     // =============================================================================
     // CONST, STATE, REF
     // =============================================================================
-    const rowCellArray: RowCellData[] = [];
+    const testId = otherProps["data-testid"] || `${id}-row`;
+    const rowCellArray: TimeTableRowCellData[] = [];
 
     // ===========================================================================
     // HELPER FUNCTIONS
@@ -55,7 +55,7 @@ export const RowBar = ({
             startTime: timetableMinTime,
             endTime: rowMinTime,
             status: "blocked",
-            customPopover: optionalProps.outsideOpHoursCellCustomPopover,
+            customPopover: outsideOpHoursCellCustomPopover,
         });
     }
 
@@ -77,15 +77,15 @@ export const RowBar = ({
                     id,
                     startTime: curr.format("HH:mm").toString(),
                     endTime: nextHour.format("HH:mm").toString(),
-                    status: undefined,
+                    status: "disabled",
                 });
                 curr = nextHour;
             } else {
                 rowCellArray.push({
                     id,
                     startTime: curr.format("HH:mm").toString(),
                     endTime: nextSlotStartTime.format("HH:mm").toString(),
-                    status: undefined,
+                    status: "disabled",
                 });
                 curr = nextSlotStartTime;
             }
@@ -99,12 +99,12 @@ export const RowBar = ({
             startTime: rowMaxTime,
             endTime: timetableMaxTime,
             status: "blocked",
-            customPopover: optionalProps.outsideOpHoursCellCustomPopover,
+            customPopover: outsideOpHoursCellCustomPopover,
         });
     }
 
     return (
-        <RowCellContainer data-testid={optionalProps["data-testid"]}>
+        <RowCellContainer data-testid={testId} {...otherProps}>
             {rowCellArray.map((cell, index) => {
                 return (
                     <RowCell
@@ -113,7 +113,6 @@ export const RowBar = ({
                         intervalWidth={intervalWidth}
                         rowBarColor={rowBarColor}
                         containerRef={containerRef}
-                        onCellClick={optionalProps.onCellClick}
                     />
                 );
             })}

diff --git a/src/timetable/timetable-row/row-cell.style.tsx b/src/timetable/timetable-row/row-cell.style.tsx
@@ -9,7 +9,7 @@ interface BlockStyleProps {
     $width: number;
     $status: CellType;
     $bgColour: string;
-    $clickableCell?: boolean;
+    $isClickable?: boolean;
 }
 
 interface BlockContainerProps {
@@ -45,7 +45,7 @@ export const Block = styled.div<BlockStyleProps>`
     border-radius: 4px;
     box-sizing: border-box;
     padding: 4px;
-    ${({ $status, $bgColour, $clickableCell }) => {
+    ${({ $status, $bgColour, $isClickable }) => {
         switch ($status) {
             case "blocked":
                 return css`
@@ -55,27 +55,27 @@ export const Block = styled.div<BlockStyleProps>`
                         #ecefef 6px 12px
                     );
                     &:hover {
-                        cursor: ${$clickableCell ? "pointer" : "not-allowed"};
+                        cursor: ${$isClickable ? "pointer" : "not-allowed"};
                     }
                 `;
             case "filled":
                 return css`
                     background: ${$bgColour};
                     &:hover {
-                        cursor: ${$clickableCell ? "pointer" : "default"};
+                        cursor: ${$isClickable ? "pointer" : "default"};
                     }
                 `;
-            case "default":
+            case "disabled":
                 return css`
+                    background: ${Color.Neutral[6]};
                     &:hover {
-                        cursor: ${$clickableCell ? "pointer" : "default"};
+                        cursor: ${$isClickable ? "pointer" : "not-allowed"};
                     }
                 `;
             default:
                 return css`
-                    background: ${Color.Neutral[6]};
                     &:hover {
-                        cursor: not-allowed;
+                        cursor: ${$isClickable ? "pointer" : "default"};
                     }
                 `;
         }

diff --git a/src/timetable/timetable-row/row-cell.tsx b/src/timetable/timetable-row/row-cell.tsx
@@ -2,7 +2,8 @@ import dayjs from "dayjs";
 import React, { MutableRefObject } from "react";
 import { PopoverTrigger, PopoverV2TriggerProps } from "../../popover-v2";
 import { DateHelper } from "../../util";
-import { ROW_CELL_GAP, ROW_INTERVAL, RowCellData } from "../types";
+import { ROW_CELL_GAP, ROW_INTERVAL } from "../const";
+import { TimeTableRowCellData } from "../types";
 import {
     Block,
     BlockContainer,
@@ -13,11 +14,10 @@ import {
     Wrapper,
 } from "./row-cell.style";
 
-interface RowCellProps extends RowCellData {
+interface RowCellProps extends TimeTableRowCellData {
     containerRef: MutableRefObject<HTMLDivElement>;
     intervalWidth: number;
     rowBarColor: string;
-    onCellClick?: (data: RowCellData, e: React.MouseEvent) => void;
 }
 
 const Component = ({
@@ -31,7 +31,7 @@ const Component = ({
     rowBarColor,
     containerRef,
     customPopover,
-    onCellClick,
+    onClick,
 }: RowCellProps) => {
     // =============================================================================
     // CONST, STATE, REF
@@ -44,13 +44,15 @@ const Component = ({
     const adjustedCellWidth = isNotAvailable
         ? totalCellWidth - ROW_CELL_GAP
         : totalCellWidth;
+    const isClickable =
+        !!onClick || (customPopover && customPopover.trigger === "click");
 
     // =============================================================================
     // EVENT HANDLERS
     // =============================================================================
     const handleCellClick = (event: React.MouseEvent) => {
-        if (status && status !== "blocked" && onCellClick) {
-            onCellClick(
+        if (onClick) {
+            onClick(
                 {
                     id,
                     startTime,
@@ -99,7 +101,7 @@ const Component = ({
         const popoverTriggerProps: PopoverV2TriggerProps = {
             position: "bottom-start",
             rootNode: containerRef,
-            offset: customPopover.offset,
+            customOffset: customPopover.offset,
             children: child,
             trigger: customPopover.trigger,
             delay: customPopover.delay,
@@ -125,7 +127,7 @@ const Component = ({
                         $width={adjustedCellWidth}
                         $status={status}
                         $bgColour={rowBarColor}
-                        $clickableCell={!!handleCellClick && !!customPopover}
+                        $isClickable={isClickable}
                         onClick={handleCellClick}
                     >
                         {title && (

diff --git a/src/timetable/timetable.style.tsx b/src/timetable/timetable.style.tsx
@@ -17,16 +17,9 @@ interface RowHeaderColumnProps {
 
 interface TimeTableContainerProps {
     $loading: boolean;
-    $width: string;
-    $height: string;
     $allRecordsLoaded: boolean;
 }
 
-interface EmptyTableContainerProps {
-    $width: string;
-    $height: string;
-}
-
 interface RowColumnHeaderProps {
     $isScrolledY: boolean;
     $isScrolledX: boolean;
@@ -48,17 +41,19 @@ interface LoaderProps {
     $isEmptyContent: boolean;
 }
 
-export const EmptyTableContainer = styled.div<EmptyTableContainerProps>`
+export const EmptyTableContainer = styled.div`
     display: grid;
+    overflow: scroll;
+    position: relative;
     grid-template-columns: 252px auto;
-    min-height: 600px; // REVIEW
-    min-width: 1000px; // REVIEW
-    height: ${(props) => props.$height};
-    width: ${(props) => props.$width};
 `;
 
 export const EmptyTableRowHeader = styled.div`
     display: flex;
+    position: sticky;
+    top: 0;
+    background-color: white;
+    z-index: 1;
     height: fit-content;
     grid-column: 1 / -1;
     border-bottom: 1px solid ${Color.Neutral[5]};
@@ -69,11 +64,7 @@ export const TimeTableContainer = styled.div<TimeTableContainerProps>`
     overflow: scroll;
     position: relative;
     grid-template-columns: 252px fit-content(100%);
-    height: ${(props) => props.$height};
-    width: ${(props) => props.$width};
     padding-bottom: ${(props) => (props.$allRecordsLoaded ? "0" : "128px")};
-    min-height: 600px; // REVIEW
-    min-width: 900px; // REVIEW
     ${(props) => {
         if (props.$loading) {
             return css`