Admin bulk action pattern #157
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,20 @@ | ||
| @use "../../base"; | ||
|
|
||
| .bookingBaseTable { | ||
| @include base.table; | ||
| cursor: pointer; | ||
|
|
||
| div { | ||
| box-shadow: none; | ||
| } | ||
|
|
||
| .extraContentContainer { | ||
| width: 100%; | ||
| } | ||
|
|
||
| .detailsContainer { | ||
| :global(.row) { | ||
| padding: 0.5rem 0; | ||
| } | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,204 @@ | ||
| import { useCallback, useState, useRef, useEffect } from "react"; | ||
| import { AutoResizer, Column, ColumnShape, RowKey } from "react-base-table"; | ||
| import { Segment, Checkbox } from "semantic-ui-react"; | ||
|
|
||
| import { | ||
| ACTION, | ||
| CREATED_AT_STRING, | ||
| EVENT_TIME_RANGE, | ||
| ID, | ||
| START_DATE_TIME_STRING, | ||
| START_TIME_MINS, | ||
| STATUS, | ||
| } from "../../constants"; | ||
| import { useGetSingleBooking } from "../../custom-hooks/api/bookings-api"; | ||
| import { useAppDispatch } from "../../redux/hooks"; | ||
| import { updateBookingsAction } from "../../redux/slices/bookings-slice"; | ||
| import { BookingData } from "../../types/bookings"; | ||
| import BookingDetailsView from "../booking-details-view"; | ||
| import BookingStatusButton from "../booking-status-button"; | ||
| import Table, { TableProps } from "../table"; | ||
| import styles from "./admin-booking-base-table.module.scss"; | ||
|
|
||
| export type BookingViewProps = BookingData & { | ||
| [START_TIME_MINS]: number; | ||
| [START_DATE_TIME_STRING]: string; | ||
| [EVENT_TIME_RANGE]: string; | ||
| [CREATED_AT_STRING]: string; | ||
| booking?: BookingData; | ||
| children: { [ID]: string; booking: BookingData }[]; | ||
| }; | ||
|
|
||
| type Props = Partial<TableProps<BookingViewProps>> & { | ||
| adminView?: boolean; | ||
| defaultStatusColumnWidth?: number; | ||
| defaultActionColumnWidth?: number; | ||
| selectedBookingIds?: Set<number>; | ||
| onSelectionChange?: (selectedIds: Set<number>) => void; | ||
| }; | ||
|
|
||
| const RowRenderer: TableProps<BookingViewProps>["rowRenderer"] = ({ | ||
| rowData: { booking }, | ||
| cells, | ||
| columns, | ||
| }: { | ||
| rowData: BookingViewProps; | ||
| cells: React.ReactNode[]; | ||
| columns: ColumnShape<BookingViewProps>; | ||
| }) => | ||
| // Only render details if there are booking details | ||
| // and the column is not the frozen column | ||
| booking && columns.length > 1 ? ( | ||
| <Segment className={styles.extraContentContainer} basic> | ||
| <BookingDetailsView | ||
| className={styles.detailsContainer} | ||
| booking={booking} | ||
| /> | ||
| </Segment> | ||
| ) : ( | ||
| cells | ||
| ); | ||
|
|
||
| function BookingBaseTable({ | ||
| adminView = false, | ||
| defaultStatusColumnWidth = 100, | ||
| defaultActionColumnWidth = 100, | ||
| selectedBookingIds = new Set(), | ||
| onSelectionChange, | ||
|
|
||
| children, | ||
| ...props | ||
| }: Props) { | ||
| const { getSingleBooking } = useGetSingleBooking(); | ||
| const dispatch = useAppDispatch(); | ||
|
|
||
| const StatusButtonRenderer: ColumnShape<BookingViewProps>["cellRenderer"] = | ||
| useCallback( | ||
| ({ rowData: { status, id } }: { rowData: BookingViewProps }) => | ||
| status && | ||
| id !== undefined && ( | ||
| <BookingStatusButton | ||
| bookingId={id} | ||
| status={status} | ||
| adminView={adminView} | ||
| /> | ||
| ), | ||
| [adminView], | ||
| ); | ||
|
|
||
| const selectedIdsRef = useRef(selectedBookingIds); | ||
|
|
||
| useEffect(() => { | ||
| selectedIdsRef.current = selectedBookingIds; | ||
| }, [selectedBookingIds]); | ||
|
|
||
| const CheckboxRenderer: ColumnShape<BookingViewProps>["cellRenderer"] = | ||
| useCallback( | ||
| ({ rowData: { id } }: { rowData: BookingViewProps }) => { | ||
| if (id === undefined) return null; | ||
| const isChecked = selectedBookingIds.has(id); | ||
| return ( | ||
| <div onClick={(e) => e.stopPropagation()}> | ||
| <Checkbox | ||
| checked={isChecked} | ||
| onChange={() => { | ||
| const currentSet = selectedIdsRef.current; | ||
| const newSelected = new Set(currentSet); | ||
| if (currentSet.has(id)) { | ||
| newSelected.delete(id); | ||
| } else { | ||
| newSelected.add(id); | ||
| } | ||
| if (onSelectionChange) { | ||
| onSelectionChange(newSelected); | ||
| } | ||
| }} | ||
| /> | ||
| </div> | ||
| ); | ||
| }, | ||
| [selectedBookingIds, onSelectionChange], | ||
| ); | ||
|
Comment on lines
+90
to
+122
There was a problem hiding this comment. The CheckboxRenderer uses a ref pattern (selectedIdsRef) to access the current selection state, but this creates a potential issue. The ref is updated in useEffect but the CheckboxRenderer callback is only recreated when selectedBookingIds or onSelectionChange changes. This means if selectedBookingIds is updated externally (e.g., by the footer's "Select all" or "Deselect all"), the CheckboxRenderer will still use the ref which points to the new Set, making the useCallback dependency array misleading. Consider removing the ref pattern and directly using selectedBookingIds in the callback, or ensure the dependencies accurately reflect what the callback uses. |
||
|
|
||
| const [expandedRowKeys, setExpandedRowKeys] = useState<RowKey[]>([]); | ||
| const onRowExpand: TableProps<BookingViewProps>["onRowExpand"] = ({ | ||
| expanded, | ||
| rowData: { id, formResponseData }, | ||
| }) => { | ||
| if (!expanded || formResponseData) { | ||
| return; | ||
| } | ||
| getSingleBooking(id) | ||
| .then((booking) => { | ||
| if (booking) { | ||
| dispatch(updateBookingsAction({ bookings: [booking] })); | ||
| } else { | ||
| console.error("Failed to update booking details for booking ID", id); | ||
| } | ||
| }) | ||
| .catch((error) => console.error(error)); | ||
| }; | ||
|
|
||
| return ( | ||
| <Segment className={styles.bookingBaseTable}> | ||
| <AutoResizer> | ||
| {({ width, height }) => ( | ||
| <Table<BookingViewProps> | ||
| width={width} | ||
| height={height} | ||
| ignoreFunctionInColumnCompare={false} | ||
| rowRenderer={RowRenderer} | ||
| estimatedRowHeight={50} | ||
| fixed | ||
| expandColumnKey={ACTION} | ||
| onRowExpand={onRowExpand} | ||
| expandedRowKeys={expandedRowKeys} | ||
| rowEventHandlers={{ | ||
| onClick: ({ rowData, rowKey, rowIndex }) => { | ||
| if (!rowData.children || rowData.children.length === 0) return; | ||
| if (expandedRowKeys.includes(rowKey)) | ||
| setExpandedRowKeys( | ||
| expandedRowKeys.filter((x) => x !== rowKey), | ||
| ); | ||
| else { | ||
| setExpandedRowKeys([...expandedRowKeys, rowKey]); | ||
| onRowExpand({ expanded: true, rowData, rowIndex, rowKey }); | ||
| } | ||
| }, | ||
| }} | ||
| {...props} | ||
| > | ||
| <Column<BookingViewProps> | ||
| key="checkbox" // hmm change to a constant? | ||
|
There was a problem hiding this comment. The inline comment "hmm change to a constant?" suggests unfinished work. The checkbox column key should be defined as a constant (similar to other column keys like ACTION, STATUS, ID, etc. from the constants file) to maintain consistency with the rest of the codebase. |
||
| title="" | ||
| width={50} | ||
| align="center" | ||
| cellRenderer={CheckboxRenderer} | ||
| resizable={false} | ||
| frozen="left" | ||
| /> | ||
|
Comment on lines
+172
to
+180
There was a problem hiding this comment. The checkbox column lacks an accessible label or aria-label. Screen readers will not be able to announce what the checkbox is for. Consider adding an aria-label to the Column component or to the Checkbox itself to indicate its purpose (e.g., "Select booking for bulk action").
Contributor
There was a problem hiding this comment. I think it's intuitive, can ignore |
||
| {children} | ||
| <Column<BookingViewProps> | ||
| key={STATUS} | ||
| title="Status" | ||
| width={defaultStatusColumnWidth} | ||
| sortable | ||
| align="center" | ||
| cellRenderer={StatusButtonRenderer} | ||
| resizable | ||
| /> | ||
| <Column<BookingViewProps> | ||
| key={ACTION} | ||
| title="" | ||
| width={defaultActionColumnWidth} | ||
| align="center" | ||
| frozen="right" | ||
| /> | ||
| </Table> | ||
| )} | ||
| </AutoResizer> | ||
| </Segment> | ||
| ); | ||
| } | ||
|
|
||
| export default BookingBaseTable; | ||
|
Comment on lines
+1
to
+205
There was a problem hiding this comment. The entire admin-booking-base-table component appears to be a near-complete duplication of the booking-base-table component, with only the addition of checkbox functionality and the ignoreFunctionInColumnCompare prop. This creates significant code duplication and maintenance burden. Consider refactoring the original booking-base-table to support the checkbox feature through props, or extract common logic into a shared component to avoid maintaining two nearly identical implementations. |
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| export type { BookingViewProps } from "./admin-booking-base-table"; | ||
| export { default } from "./admin-booking-base-table"; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,99 @@ | ||||||
| .footerContainer { | ||||||
| /* Positioning */ | ||||||
| position: fixed !important; | ||||||
| bottom: 40px !important; | ||||||
| left: 50% !important; | ||||||
| transform: translateX(-50%) !important; | ||||||
|
|
||||||
| /* Layout constraints */ | ||||||
| top: auto !important; | ||||||
| height: auto !important; | ||||||
| width: auto !important; | ||||||
| min-width: 0 !important; | ||||||
|
|
||||||
| /* Visuals */ | ||||||
| background-color: #ffffff; | ||||||
| border: 1px solid #dfe1e6; | ||||||
| box-shadow: 0 4px 12px rgba(9, 30, 66, 0.15), 0 0 1px rgba(9, 30, 66, 0.31); | ||||||
| border-radius: 6px; | ||||||
|
|
||||||
| /* Flexbox settings */ | ||||||
| display: flex; | ||||||
| align-items: center; | ||||||
| justify-content: center; | ||||||
|
|
||||||
| gap: 20px; | ||||||
| padding: 14px 24px; | ||||||
|
|
||||||
| z-index: 9999; | ||||||
| animation: slideUp 0.3s ease-out; | ||||||
| } | ||||||
|
|
||||||
| .selectionGroup { | ||||||
| display: flex; | ||||||
| align-items: center; | ||||||
| gap: 6px; | ||||||
| padding-right: 20px; | ||||||
| border-right: 1px solid #dfe1e6; | ||||||
| } | ||||||
|
|
||||||
| .countBadge { | ||||||
| color: #172b4d; | ||||||
| font-weight: 700; | ||||||
| font-size: 14px; | ||||||
| display: flex; | ||||||
| align-items: center; | ||||||
| justify-content: center; | ||||||
| } | ||||||
|
|
||||||
| .selectionText { | ||||||
| color: #172b4d; | ||||||
| font-size: 14px; | ||||||
| font-weight: 500; | ||||||
| } | ||||||
|
|
||||||
| /* --- NEW STYLES --- */ | ||||||
|
|
||||||
| .separator { | ||||||
| color: #dfe1e6; /* Subtle grey divider */ | ||||||
| margin: 0 8px; | ||||||
| font-size: 18px; /* Slightly taller than text */ | ||||||
| line-height: 1; | ||||||
| } | ||||||
|
|
||||||
| .selectAllBtn { | ||||||
| /* Override default Button styles to look like a link */ | ||||||
| background: none !important; | ||||||
| border: none !important; | ||||||
| padding: 0 !important; | ||||||
| margin: 0 !important; | ||||||
| box-shadow: none !important; | ||||||
|
|
||||||
| color: #0052cc !important; /* Standard "Link Blue" */ | ||||||
| font-weight: 600 !important; | ||||||
| font-size: 14px !important; | ||||||
| cursor: pointer; | ||||||
|
|
||||||
| &:hover { | ||||||
| text-decoration: underline; | ||||||
| background: none !important; | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| /* ------------------ */ | ||||||
|
|
||||||
| .buttonGroup { | ||||||
| display: flex; | ||||||
| gap: 12px; | ||||||
| } | ||||||
|
|
||||||
| .actionBtn { | ||||||
| margin: 0 !important; | ||||||
| box-shadow: none !important; | ||||||
| font-weight: 600 !important; | ||||||
| } | ||||||
|
|
||||||
| @keyframes slideUp { | ||||||
| from { transform: translate(-50%, 100%); opacity: 0; } | ||||||
|
There was a problem hiding this comment. The slideUp animation uses translate(-50%, 100%) which moves the element down by 100% before animating up. However, the footer is positioned at bottom: 40px, which means the element might not be fully off-screen at the start of the animation, potentially causing a visual glitch. Consider using translateY(calc(100% + 40px)) to ensure the element starts completely off-screen.
Suggested change
|
||||||
| to { transform: translate(-50%, 0); opacity: 1; } | ||||||
| } | ||||||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
The checkbox is wrapped in a div with onClick stopPropagation, which prevents the row click event from firing when the checkbox is clicked. However, this only stops mouse click propagation. Keyboard users using Space or Enter to toggle the checkbox may still trigger the row expansion. Consider also adding onKeyDown handler to stop keyboard event propagation, or ensure the Checkbox component itself handles keyboard events appropriately.
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.
@Shum-ster verify this