codeant-ai-for-open-source[bot] commented on code in PR #43472:
URL: https://github.com/apache/superset/pull/43472#discussion_r3858206365
##########
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:
Yes. The detection should account for the horizontal scrollbar’s height
before deciding whether the vertical scrollbar is present.
Use the Grid’s actual column width to detect horizontal overflow, then
reduce the effective height by the scrollbar footprint:
```ts
const scrollBarSize = getScrollBarSize();
const hasHorizontalScroll = totalWidth > tableWidth;
const effectiveHeight =
availableHeight - (hasHorizontalScroll ? scrollBarSize : 0);
const hasVerticalScroll = rowCount * cellSize > effectiveHeight;
const columnSizingWidth = Math.max(
tableWidth - (hasVerticalScroll ? scrollBarSize : 0),
0,
);
```
Because `totalWidth` currently depends on `columnSizingWidth`, the width
calculation should be structured in two passes (or otherwise determine
horizontal overflow from the unadjusted column widths) to avoid a circular
dependency. This covers the edge case where rows exactly fit the nominal height
but a horizontal scrollbar reduces the Grid’s client height and causes vertical
overflow.
--
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]
