sadpandajoe commented on code in PR #43472:
URL: https://github.com/apache/superset/pull/43472#discussion_r4175298705
##########
superset-frontend/packages/superset-ui-core/src/components/Table/VirtualTable.test.tsx:
##########
@@ -120,3 +124,50 @@ test('cell render functions receive their row data via
cellProps rather than a s
expect(screen.getByText('rendered:Number')).toBeInTheDocument();
});
});
+
+test('reserves space for the body Grid vertical scrollbar when sizing header
columns', async () => {
+ const resizeSpy = jest.spyOn(resizeDetector, 'useResizeDetector');
+ let resized = false;
+ resizeSpy.mockImplementation(props => {
+ if (props?.onResize && !resized) {
+ resized = true;
+ props.onResize(400);
+ }
+ return { ref: { current: undefined } };
+ });
+ jest.mocked(getScrollBarSize).mockReturnValue(20);
+
+ const manyRows: BasicData[] = Array.from({ length: 50 }, (_, i) => ({
+ columnName: `Row ${i}`,
+ columnType: 'Numerical',
+ }));
+ const columnsNoWidth: ColumnsType<BasicData> = [
+ { title: 'Column Name', dataIndex: 'columnName', key: 'columnName' },
+ { title: 'Column Type', dataIndex: 'columnType', key: 'columnType' },
+ ];
+
+ const { container } = render(
+ <Table
+ columns={columnsNoWidth}
+ data={manyRows}
+ virtualize
+ height={100}
+ usePagination={false}
+ />,
+ );
+
+ await waitFor(() => {
+ expect(container.querySelectorAll('col').length).toBe(2);
+ });
+
+ // Enough rows at the default row height to overflow a 100px-tall Grid,
+ // so the 20px scrollbar reservation should come out of the 400px
+ // container width rather than being added on top of it.
+ const totalColWidth = Array.from(container.querySelectorAll('col')).reduce(
+ (sum, col) => sum + parseFloat(col.style.width || '0'),
+ 0,
+ );
+ expect(totalColWidth).toBe(380);
Review Comment:
This can pass with misaligned header/body columns because only our helper
reports a 20px scrollbar; rc-table's own measurement stays zero here, so its
header gutter and final-column adjustment never participate. Could this cover
the 400px overflowing-table case with both measurements set to 20px and assert
corresponding header/Grid cell widths rather than just the header sum?
--
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]
