Bl 11194 table fixed columns drag resize improvements#172
Bl 11194 table fixed columns drag resize improvements#172bradleyfrance wants to merge 4 commits intomainfrom
Conversation
…o right edge - SortableContext now uses draggableColumnIds (excludes fixedEndColIds) so fixed-end columns cannot be dragged or displaced - Strip fixedEndColIds/fixedStartColIds from persisted column order to prevent stale localStorage state - Resize handle line uses justifyContent flex-end so blue indicator aligns to exact column boundary - Elevate th zIndex on resize hover/active to prevent adjacent column overlap Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Improves the TanStack-based data table’s column interactions by adding resize handles/visual indicators, persisting column sizing, and refining table scroll + fixed-end column behavior.
Changes:
- Add column resizing support (incl. localStorage persistence) and resize visual indicators.
- Adjust header/cell rendering and styling to better support resizing + fixed-end sticky columns.
- Add custom scrollbar styling for horizontally scrollable data table wrappers.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 8 comments.
| File | Description |
|---|---|
src/misc/tanstack-table/TableComponents.tsx |
Updates draggable header rendering and adds a dedicated resize handle + hover/resizing z-index styling. |
src/misc/tanstack-table/Table.tsx |
Adds persisted columnSizing, enables TanStack column resizing, and refactors header/body rendering for fixed layout + sticky fixed-end columns. |
src/index.css |
Adds data-table-scroll scrollbar styling for the table wrapper. |
Comments suppressed due to low confidence (1)
src/misc/tanstack-table/Table.tsx:108
getInitialColumnOrdercomputesvalidFixedEnd(filtered to columns that actually exist) but the fallback return path still appends the unfilteredfixedEndColIds. This can leave unknown column IDs incolumnOrder, which can lead to inconsistent ordering/state when a fixed-end column isn't present for a given table. Use the validated list (and consider similarly validatingfixedStartColIds) in the final returned order.
const colIds = [...allColIds];
const draggable = colIds.filter(
(id) => !fixedStartColIds.includes(id) && !fixedEndColIds.includes(id)
);
return [...fixedStartColIds, ...draggable, ...fixedEndColIds];
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| className={clsx( | ||
| 'py-3 text-text-pri transition-all duration-200', | ||
| 'relative py-4 text-text-pri transition-all duration-200', | ||
| 'last:[>td]:justify-center', |
There was a problem hiding this comment.
DraggableColumnHeader no longer renders a direct td child inside the th (it now renders a div), but the class string still includes last:[>td]:justify-center. That selector will never match now, so the intended styling won’t apply. Update the selector to target the current child element (or remove it if no longer needed).
| 'last:[>td]:justify-center', | |
| 'last:[&>div]:justify-center', |
| /* Thicker horizontal scrollbar for data tables */ | ||
| .data-table-scroll { | ||
| scrollbar-width: thin; | ||
| scrollbar-color: #cbd5e1 #f1f5f9; | ||
| } |
There was a problem hiding this comment.
The comment says this makes the horizontal scrollbar “Thicker”, but scrollbar-width: thin makes it thinner in Firefox. Either adjust the comment to match the implementation, or change the Firefox scrollbar setting if the intent is genuinely a thicker scrollbar there too.
| const colWidth = | ||
| h.column.columnDef.maxSize !== undefined && | ||
| h.column.columnDef.maxSize < | ||
| Number.MAX_SAFE_INTEGER | ||
| ? h.getSize() | ||
| : Math.max( | ||
| h.column.columnDef.minSize ?? 20, | ||
| 80 | ||
| ); |
There was a problem hiding this comment.
Sticky right offsets are calculated using a fallback Math.max(minSize, 80) whenever columnDef.maxSize is undefined or very large. That means the sticky positioning can be wrong whenever a fixed-end column’s actual size differs from 80px (including after user resizing). Use the column’s actual computed size (e.g. getSize()) for offset accumulation so sticky columns don’t overlap/gap.
| const colWidth = | |
| h.column.columnDef.maxSize !== undefined && | |
| h.column.columnDef.maxSize < | |
| Number.MAX_SAFE_INTEGER | |
| ? h.getSize() | |
| : Math.max( | |
| h.column.columnDef.minSize ?? 20, | |
| 80 | |
| ); | |
| const colWidth = h.getSize(); |
| const colWidth = | ||
| c.column.columnDef.maxSize !== undefined && | ||
| c.column.columnDef.maxSize < Number.MAX_SAFE_INTEGER | ||
| ? c.column.getSize() | ||
| : Math.max(c.column.columnDef.minSize ?? 20, 80); |
There was a problem hiding this comment.
Same as the header offset logic: sticky cell right offsets fall back to Math.max(minSize, 80) when maxSize isn’t set, which can misposition fixed-end sticky columns (especially after resize). Accumulate offsets using the column’s actual getSize() so the sticky right value matches real rendered widths.
| const colWidth = | |
| c.column.columnDef.maxSize !== undefined && | |
| c.column.columnDef.maxSize < Number.MAX_SAFE_INTEGER | |
| ? c.column.getSize() | |
| : Math.max(c.column.columnDef.minSize ?? 20, 80); | |
| const colWidth = c.column.getSize(); |
| ); | ||
| setColumnOrder([...fixedStartColIds, ...base, ...fixedEndColIds]); | ||
| } | ||
| }, [initialColumnOrder]); |
There was a problem hiding this comment.
This useEffect derives columnOrder using fixedStartColIds/fixedEndColIds but those arrays are not included in the dependency list. If the pinned columns change between renders, columnOrder can become stale/misaligned. Include the fixed column ID arrays (and any other referenced values) in the deps or memoize them to stable references.
| }, [initialColumnOrder]); | |
| }, [initialColumnOrder, fixedStartColIds, fixedEndColIds]); |
| className={`${itemProps?.table?.className} text-text-pri`} | ||
| style={{ | ||
| ...(itemProps?.table?.style || {}), | ||
| minWidth: tableMinWidth, |
There was a problem hiding this comment.
Table is rendered with w-full and tableLayout: 'fixed', but without setting an explicit width that matches tableMinWidth. Combined with percentage-based header widths, this can cause resized column pixel sizes (and the persisted columnSizing values) to scale with container width instead of staying at the intended pixel widths. Consider setting the table width to the computed min width (so it overflows horizontally) and/or applying column widths in px rather than percentages.
| minWidth: tableMinWidth, | |
| minWidth: tableMinWidth, | |
| width: tableMinWidth, |
| width: `${( | ||
| (header.getSize() / totalColWidth) * | ||
| 100 | ||
| ).toFixed(4)}%`, |
There was a problem hiding this comment.
Header widths are being applied as percentages derived from header.getSize(). Because the table itself can expand beyond tableMinWidth (it’s w-full), the actual rendered pixel widths will scale, which undermines pixel-precise resizing and localStorage persistence. Prefer applying getSize() directly as a px width (or ensure the table width is constrained to the sum of column sizes).
| width: `${( | |
| (header.getSize() / totalColWidth) * | |
| 100 | |
| ).toFixed(4)}%`, | |
| width: `${header.getSize()}px`, |
| const isBeforeFixedEnd = | ||
| !!nextHeader && | ||
| fixedEndColIds.includes(nextHeader.column.id); |
There was a problem hiding this comment.
isBeforeFixedEnd is computed but never used. This adds dead code and makes the intent harder to follow—either remove it or use it to drive the styling/behavior it was introduced for.
| const isBeforeFixedEnd = | |
| !!nextHeader && | |
| fixedEndColIds.includes(nextHeader.column.id); |
Feat: Improved column resize behaviour, visual indicators, and column separator styling across the Tokens, Blogs, Blog Posts tables, SEO table, Companies under User as well as User tables - Need further help with some of the styling for this, but most of it looks good.
Another PR regarding the same table/colums for the HB/CMS
https://broadlume.atlassian.net/browse/BL-11194