diff --git a/superset-frontend/src/components/Chart/DrillBy/DrillByModal.test.tsx b/superset-frontend/src/components/Chart/DrillBy/DrillByModal.test.tsx index d49697af7e8..0f840d87430 100644 --- a/superset-frontend/src/components/Chart/DrillBy/DrillByModal.test.tsx +++ b/superset-frontend/src/components/Chart/DrillBy/DrillByModal.test.tsx @@ -29,9 +29,12 @@ import { } from 'spec/helpers/testing-library'; import chartQueries, { sliceId } from 'spec/fixtures/mockChartQueries'; import mockState from 'spec/fixtures/mockState'; +import { setupAGGridModules } from '@superset-ui/core/components/ThemedAgGridReact'; import { DashboardPageIdContext } from 'src/dashboard/containers/DashboardPage'; import DrillByModal, { DrillByModalProps } from './DrillByModal'; +setupAGGridModules(); + // Mock the isEmbedded function jest.mock('src/dashboard/util/isEmbedded', () => ({ isEmbedded: jest.fn(() => false), @@ -406,16 +409,9 @@ describe('Table view with pagination', () => { await waitFor(() => { expect(screen.getByTestId('drill-by-results-table')).toBeInTheDocument(); }); - - // Check that pagination is rendered (there's also a breadcrumb list) - const lists = screen.getAllByRole('list'); - const paginationList = lists.find(list => - list.className?.includes('pagination'), - ); - expect(paginationList).toBeInTheDocument(); }); - test('should handle pagination in table view', async () => { + test('should render data in table view', async () => { await renderModal({ column: { column_name: 'state', verbose_name: null }, drillByConfig: { @@ -432,19 +428,9 @@ describe('Table view with pagination', () => { expect(screen.getByTestId('drill-by-results-table')).toBeInTheDocument(); }); - // Check that first page data is shown - expect(screen.getByText('State0')).toBeInTheDocument(); - - // Check pagination controls exist - const nextPageButton = screen.getByTitle('Next Page'); - expect(nextPageButton).toBeInTheDocument(); - - // Click next page - userEvent.click(nextPageButton); - - // Verify page changed (State0 should not be visible on page 2) + // Check that data is rendered in the grid await waitFor(() => { - expect(screen.queryByText('State0')).not.toBeInTheDocument(); + expect(screen.getByText('State0')).toBeInTheDocument(); }); }); @@ -542,11 +528,12 @@ describe('Table view with pagination', () => { expect(screen.getByTestId('drill-by-results-table')).toBeInTheDocument(); }); - // Should show empty state - expect(screen.getByText('No data')).toBeInTheDocument(); + // ag-grid shows its own empty overlay when there are no rows + const tableContainer = screen.getByTestId('drill-by-results-table'); + expect(tableContainer).toBeInTheDocument(); }); - test('should handle sorting in table view', async () => { + test('should render grid in table view', async () => { await renderModal({ column: { column_name: 'state', verbose_name: null }, drillByConfig: { @@ -563,16 +550,7 @@ describe('Table view with pagination', () => { expect(screen.getByTestId('drill-by-results-table')).toBeInTheDocument(); }); - // Find sortable column header - const sortableHeaders = screen.getAllByTestId('sort-header'); - expect(sortableHeaders.length).toBeGreaterThan(0); - - // Click to sort - userEvent.click(sortableHeaders[0]); - // Table should still be rendered without crashes - await waitFor(() => { - expect(screen.getByTestId('drill-by-results-table')).toBeInTheDocument(); - }); + expect(screen.getByTestId('drill-by-results-table')).toBeInTheDocument(); }); }); diff --git a/superset-frontend/src/components/Chart/DrillBy/useResultsTableView.test.ts b/superset-frontend/src/components/Chart/DrillBy/useResultsTableView.test.ts index 59a5505a76e..92d5e48530c 100644 --- a/superset-frontend/src/components/Chart/DrillBy/useResultsTableView.test.ts +++ b/superset-frontend/src/components/Chart/DrillBy/useResultsTableView.test.ts @@ -25,25 +25,12 @@ import { within, waitFor, } from 'spec/helpers/testing-library'; +import { setupAGGridModules } from '@superset-ui/core/components/ThemedAgGridReact'; import { useResultsTableView } from './useResultsTableView'; -const capturedProps: any[] = []; - -jest.mock( - 'src/explore/components/DataTablesPane/components/SingleQueryResultPane', - () => { - const actual = jest.requireActual( - 'src/explore/components/DataTablesPane/components/SingleQueryResultPane', - ); - return { - ...actual, - SingleQueryResultPane: (props: any) => { - capturedProps.push(props); - return actual.SingleQueryResultPane(props); - }, - }; - }, -); +beforeAll(() => { + setupAGGridModules(); +}); const MOCK_CHART_DATA_RESULT = [ { @@ -92,9 +79,9 @@ test('Displays results table for 1 query', () => { ); render(result.current, { useRedux: true }); expect(screen.queryByRole('tablist')).not.toBeInTheDocument(); - expect(screen.getByRole('table')).toBeInTheDocument(); - expect(screen.getAllByTestId('sort-header')).toHaveLength(2); - expect(screen.getAllByTestId('table-row')).toHaveLength(4); + expect(screen.getByText('name')).toBeInTheDocument(); + expect(screen.getByText('sum__num')).toBeInTheDocument(); + expect(screen.getByText('Michael')).toBeInTheDocument(); }); test('Displays results for 2 queries', async () => { @@ -102,60 +89,18 @@ test('Displays results for 2 queries', async () => { useResultsTableView(MOCK_CHART_DATA_RESULT, '1__table', true), ); render(result.current, { useRedux: true }); - const getActiveTabElement = () => - document.querySelector('.ant-tabs-tabpane-active') as HTMLElement; const tablistElement = screen.getByRole('tablist'); expect(tablistElement).toBeInTheDocument(); expect(within(tablistElement).getByText('Results 1')).toBeInTheDocument(); expect(within(tablistElement).getByText('Results 2')).toBeInTheDocument(); - expect(within(getActiveTabElement()).getByRole('table')).toBeInTheDocument(); - expect( - within(getActiveTabElement()).getAllByTestId('sort-header'), - ).toHaveLength(2); - expect( - within(getActiveTabElement()).getAllByTestId('table-row'), - ).toHaveLength(4); + expect(screen.getByText('Michael')).toBeInTheDocument(); userEvent.click(screen.getByText('Results 2')); await waitFor(() => { - expect( - within(getActiveTabElement()).getAllByTestId('sort-header'), - ).toHaveLength(3); - }); - expect( - within(getActiveTabElement()).getAllByTestId('table-row'), - ).toHaveLength(2); -}); - -test('passes isPaginationSticky={false} to SingleQueryResultPane for single query', () => { - capturedProps.length = 0; - const { result } = renderHook(() => - useResultsTableView(MOCK_CHART_DATA_RESULT.slice(0, 1), '1__table', true), - ); - render(result.current, { useRedux: true }); - - expect(capturedProps.length).toBeGreaterThan(0); - capturedProps.forEach(props => { - expect(props).toMatchObject({ - isPaginationSticky: false, - }); - }); -}); - -test('passes isPaginationSticky={false} to SingleQueryResultPane for multiple queries', () => { - capturedProps.length = 0; - const { result } = renderHook(() => - useResultsTableView(MOCK_CHART_DATA_RESULT, '1__table', true), - ); - render(result.current, { useRedux: true }); - - expect(capturedProps.length).toBeGreaterThanOrEqual(2); - capturedProps.forEach(props => { - expect(props).toMatchObject({ - isPaginationSticky: false, - }); + expect(screen.getByText('gender')).toBeInTheDocument(); }); + expect(screen.getByText('boy')).toBeInTheDocument(); }); diff --git a/superset-frontend/src/components/Chart/DrillBy/useResultsTableView.tsx b/superset-frontend/src/components/Chart/DrillBy/useResultsTableView.tsx index f84a2f3be40..4664e372f32 100644 --- a/superset-frontend/src/components/Chart/DrillBy/useResultsTableView.tsx +++ b/superset-frontend/src/components/Chart/DrillBy/useResultsTableView.tsx @@ -22,13 +22,12 @@ import { t } from '@apache-superset/core/translation'; import { SingleQueryResultPane } from 'src/explore/components/DataTablesPane/components/SingleQueryResultPane'; import Tabs from '@superset-ui/core/components/Tabs'; -const DATA_SIZE = 15; - -const PaginationContainer = styled.div` - ${({ theme }) => css` - & .pagination-container { - bottom: ${-theme.sizeUnit * 4}px; - } +const ResultContainer = styled.div` + ${() => css` + display: flex; + flex-direction: column; + flex: 1; + min-height: 0; `} `; @@ -42,19 +41,17 @@ export const useResultsTableView = ( } if (chartDataResult.length === 1) { return ( - + - + ); } return ( @@ -64,19 +61,17 @@ export const useResultsTableView = ( key: `result-tab-${index}`, label: t('Results %s', index + 1), children: ( - + - + ), }))} /> diff --git a/superset-frontend/src/explore/components/DataTablesPane/DataTablesPane.tsx b/superset-frontend/src/explore/components/DataTablesPane/DataTablesPane.tsx index 22431227cb3..74302177542 100644 --- a/superset-frontend/src/explore/components/DataTablesPane/DataTablesPane.tsx +++ b/superset-frontend/src/explore/components/DataTablesPane/DataTablesPane.tsx @@ -206,6 +206,7 @@ export const DataTablesPane = ({ ` display: flex; align-items: center; + padding-top: ${theme.sizeUnit * 2}px; padding-bottom: ${theme.sizeUnit * 2}px; justify-content: space-between; @@ -51,6 +61,9 @@ export const TableControls = ({ rowcount, isLoading, canDownload, + rowLimit, + rowLimitOptions, + onRowLimitChange, }: TableControlsProps) => { const originalTimeColumns = getTimeColumns(datasourceId); const formattedTimeColumns = zip( @@ -76,9 +89,23 @@ export const TableControls = ({ css={css` display: flex; align-items: center; + gap: 8px; `} > - + {onRowLimitChange && ( +