fix(data-table): render separator rows with valid table structure#28499
fix(data-table): render separator rows with valid table structure#28499abdulbaqui17 wants to merge 3 commits intocalcom:mainfrom
Conversation
|
|
There was a problem hiding this comment.
Pull request overview
This PR aims to fix invalid HTML table nesting in the web DataTable separator rows (eliminating React validateDOMNesting warnings) by ensuring separator content renders inside a table cell and spans the table width. It also includes additional changes related to attendee email time format fallback and reschedule redirect duration propagation.
Changes:
- Fix
SeparatorRowRendererto render a<td>(TableCell) within<tr>and span across columns. - Add attendee email rendering fallback so attendee
timeFormatdefaults to organizer’s when missing, with a unit test. - Add reschedule redirect behavior to include
durationin the destination query string, with SSR tests; add a small unit test forgetMultipleDurationValue.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/features/bookings/lib/get-booking.test.ts | Adds unit coverage for getMultipleDurationValue defaulting behavior. |
| packages/emails/templates/attendee-scheduled-email.ts | Ensures attendee timeFormat falls back to organizer’s for email rendering. |
| packages/emails/email-manager.test.ts | Adds test asserting renderEmail receives attendee timeFormat fallback. |
| apps/web/modules/data-table/components/DataTable.tsx | Fixes separator row DOM structure by rendering a TableCell with colSpan. |
| apps/web/lib/reschedule/[uid]/getServerSideProps.ts | Adds duration query param to reschedule redirect based on booking time range. |
| apps/web/lib/reschedule/[uid]/getServerSideProps.test.ts | Adds tests verifying duration is preserved/included in reschedule redirects. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| <SeparatorRowRenderer | ||
| separator={row.original as SeparatorRow} | ||
| className={separatorClassName} | ||
| colSpan={table.getAllColumns().length} |
| const originalBookingDurationInMinutes = Math.round( | ||
| (booking.endTime.getTime() - booking.startTime.getTime()) / (1000 * 60) | ||
| ); | ||
|
|
||
| if (originalBookingDurationInMinutes > 0) { | ||
| destinationUrlSearchParams.set("duration", String(originalBookingDurationInMinutes)); | ||
| } |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 6 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/web/modules/data-table/components/DataTable.tsx">
<violation number="1" location="apps/web/modules/data-table/components/DataTable.tsx:399">
P2: Separator row colSpan uses all columns instead of visible leaf columns, causing misalignment when columns are hidden or grouped.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| <SeparatorRowRenderer | ||
| separator={row.original as SeparatorRow} | ||
| className={separatorClassName} | ||
| colSpan={table.getAllColumns().length} |
There was a problem hiding this comment.
P2: Separator row colSpan uses all columns instead of visible leaf columns, causing misalignment when columns are hidden or grouped.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/web/modules/data-table/components/DataTable.tsx, line 399:
<comment>Separator row colSpan uses all columns instead of visible leaf columns, causing misalignment when columns are hidden or grouped.</comment>
<file context>
@@ -384,7 +393,11 @@ function DataTableBody<TData>({
+ <SeparatorRowRenderer
+ separator={row.original as SeparatorRow}
+ className={separatorClassName}
+ colSpan={table.getAllColumns().length}
+ />
</TableRow>
</file context>
| colSpan={table.getAllColumns().length} | |
| colSpan={table.getVisibleLeafColumns().length} |
Summary
tr > divstructure withtr > tdand setcolSpanto the current column countWhy
SeparatorRowRenderercurrently renders adivdirectly undertr, which violates HTML table nesting rules and triggers ReactvalidateDOMNestingwarnings.Validation
TZ=UTC yarn vitest run apps/web/modules/data-table/components/filters/ColumnVisibilityButton.test.tsxFixes #28184