sadpandajoe commented on code in PR #43472:
URL: https://github.com/apache/superset/pull/43472#discussion_r3858203396
##########
superset-frontend/packages/superset-ui-core/src/components/Table/VirtualTable.tsx:
##########
@@ -147,6 +149,28 @@ const VirtualTable = <RecordType extends object>(
const { ref } = useResizeDetector({ onResize });
const theme = useTheme();
+ const cellSize = size === TableSize.Middle ? MIDDLE : SMALL;
+
+ // The header is rendered by antd as a plain `<thead>`, separate from the
+ // react-window `Grid` that renders the body. When the Grid's own vertical
+ // scrollbar appears, it eats into the body's visible column width without
+ // shrinking the header - misaligning the two. Reserve that same space when
+ // sizing columns so both stay in sync (mirrors the fix applied to
+ // `useSticky.tsx` for the non-virtualized table's sticky header).
+ const availableHeight = height || (scroll?.y as number) || 0;
+ // `dataSource` is the full (unpaginated) dataset - when pagination is on,
+ // only a page's worth of rows actually mounts in the Grid at a time, so
+ // that (not the full dataset length) is what determines whether its
+ // vertical scrollbar appears.
+ const pageSize =
+ pagination && typeof pagination === 'object'
+ ? pagination.pageSize
+ : undefined;
+ const rowCount = pageSize ?? dataSource?.length ?? 0;
Review Comment:
When a paginated table's final page contains fewer rows than
`pagination.pageSize`, this reserves scrollbar width for the configured page
capacity even though the mounted rows fit. The header then becomes narrower
than the body and reintroduces the alignment problem on short pages. Should
this use the actual mounted row count when determining vertical overflow?
##########
superset-frontend/packages/superset-ui-core/src/components/Table/VirtualTable.tsx:
##########
@@ -147,6 +149,28 @@ const VirtualTable = <RecordType extends object>(
const { ref } = useResizeDetector({ onResize });
const theme = useTheme();
+ const cellSize = size === TableSize.Middle ? MIDDLE : SMALL;
+
+ // The header is rendered by antd as a plain `<thead>`, separate from the
+ // react-window `Grid` that renders the body. When the Grid's own vertical
+ // scrollbar appears, it eats into the body's visible column width without
+ // shrinking the header - misaligning the two. Reserve that same space when
+ // sizing columns so both stay in sync (mirrors the fix applied to
+ // `useSticky.tsx` for the non-virtualized table's sticky header).
+ const availableHeight = height || (scroll?.y as number) || 0;
+ // `dataSource` is the full (unpaginated) dataset - when pagination is on,
+ // only a page's worth of rows actually mounts in the Grid at a time, so
+ // that (not the full dataset length) is what determines whether its
+ // vertical scrollbar appears.
+ const pageSize =
+ pagination && typeof pagination === 'object'
+ ? pagination.pageSize
+ : undefined;
+ const rowCount = pageSize ?? dataSource?.length ?? 0;
+ const hasVerticalScroll = rowCount * cellSize > availableHeight;
+ const scrollBarSize = hasVerticalScroll ? getScrollBarSize() : 0;
Review Comment:
Agreed—the horizontal scrollbar reduces the Grid's client height. A page
whose rows fit the outer height can then gain a vertical scrollbar with no
header reservation, recreating the alignment problem. Should the detection
include the horizontal scrollbar footprint?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
