diff --git a/superset-frontend/cypress-base/cypress/e2e/dashboard/drillby.test.ts b/superset-frontend/cypress-base/cypress/e2e/dashboard/drillby.test.ts deleted file mode 100644 index edc44a6fdb38..000000000000 --- a/superset-frontend/cypress-base/cypress/e2e/dashboard/drillby.test.ts +++ /dev/null @@ -1,766 +0,0 @@ -/** - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ -// eslint-disable-next-line import/no-extraneous-dependencies -import { Interception } from 'cypress/types/net-stubbing'; -import { waitForChartLoad } from 'cypress/utils'; -import { SUPPORTED_CHARTS_DASHBOARD } from 'cypress/utils/urls'; -import { - openTopLevelTab, - SUPPORTED_TIER1_CHARTS, - SUPPORTED_TIER2_CHARTS, -} from './utils'; -import { - interceptExploreJson, - interceptV1ChartData, - interceptFormDataKey, -} from '../explore/utils'; - -const interceptDrillInfo = () => { - cy.intercept('GET', '**/api/v1/dataset/*/drill_info/*', { - statusCode: 200, - body: { - result: { - id: 1, - changed_on_humanized: '2 days ago', - created_on_humanized: 'a week ago', - table_name: 'birth_names', - changed_by: { - first_name: 'Admin', - last_name: 'User', - }, - created_by: { - first_name: 'Admin', - last_name: 'User', - }, - owners: [ - { - first_name: 'Admin', - last_name: 'User', - }, - ], - columns: [ - { - column_name: 'gender', - verbose_name: null, - }, - { - column_name: 'state', - verbose_name: null, - }, - { - column_name: 'name', - verbose_name: null, - }, - { - column_name: 'ds', - verbose_name: null, - }, - ], - }, - }, - }).as('drillInfo'); -}; - -const closeModal = () => { - cy.get('body').then($body => { - if ($body.find('[data-test="close-drill-by-modal"]').length) { - cy.getBySel('close-drill-by-modal').click({ force: true }); - } - }); -}; - -const openTableContextMenu = ( - cellContent: string, - tableSelector = "[data-test-viz-type='table']", -) => { - cy.get(tableSelector).scrollIntoView(); - cy.get(tableSelector).contains(cellContent).first().rightclick(); -}; - -const drillBy = (targetDrillByColumn: string, isLegacy = false) => { - if (isLegacy) { - interceptExploreJson('legacyData'); - } else { - interceptV1ChartData(); - } - - cy.get('.ant-dropdown:not(.ant-dropdown-hidden)', { timeout: 15000 }) - .should('be.visible') - .find("[role='menu'] [role='menuitem']") - .contains(/^Drill by$/) - .trigger('mouseover', { force: true }); - - cy.get( - '.ant-dropdown-menu-submenu:not(.ant-dropdown-menu-submenu-hidden) [data-test="drill-by-submenu"]', - { timeout: 15000 }, - ) - .should('be.visible') - .find('[role="menuitem"]') - .contains(new RegExp(`^${targetDrillByColumn}$`)) - .click(); - - cy.get( - '.ant-dropdown-menu-submenu:not(.ant-dropdown-menu-submenu-hidden) [data-test="drill-by-submenu"]', - ).trigger('mouseout', { clientX: 0, clientY: 0, force: true }); - - cy.get( - '.ant-dropdown-menu-submenu:not(.ant-dropdown-menu-submenu-hidden) [data-test="drill-by-submenu"]', - ).should('not.exist'); - - if (isLegacy) { - return cy.wait('@legacyData'); - } - return cy.wait('@v1Data'); -}; - -const verifyExpectedFormData = ( - interceptedRequest: Interception, - // eslint-disable-next-line @typescript-eslint/no-explicit-any - expectedFormData: Record, -) => { - const actualFormData = interceptedRequest.request.body?.form_data; - Object.entries(expectedFormData).forEach(([key, val]) => { - expect(actualFormData?.[key]).to.eql(val); - }); -}; - -const testEchart = ( - vizType: string, - chartName: string, - drillClickCoordinates: [[number, number], [number, number]], - furtherDrillDimension = 'name', -) => { - cy.get(`[data-test-viz-type='${vizType}'] canvas`).then($canvas => { - // click 'boy' - cy.wrap($canvas).scrollIntoView(); - cy.wrap($canvas).trigger( - 'mouseover', - drillClickCoordinates[0][0], - drillClickCoordinates[0][1], - ); - cy.wrap($canvas).rightclick( - drillClickCoordinates[0][0], - drillClickCoordinates[0][1], - ); - - drillBy('state').then(intercepted => { - verifyExpectedFormData(intercepted, { - groupby: ['state'], - adhoc_filters: [ - { - clause: 'WHERE', - comparator: 'boy', - expressionType: 'SIMPLE', - operator: '==', - operatorId: 'EQUALS', - subject: 'gender', - }, - ], - }); - }); - - cy.getBySel(`"Drill by: ${chartName}-modal"`).as('drillByModal'); - - cy.get('@drillByModal') - .find('.draggable-trigger') - .should('contain', chartName); - - cy.get('@drillByModal') - .find('.ant-breadcrumb') - .should('be.visible') - .and('contain', 'gender (boy)') - .and('contain', '/') - .and('contain', 'state'); - - cy.get('@drillByModal') - .find('[data-test="drill-by-chart"]') - .should('be.visible'); - - // further drill - cy.get(`[data-test="drill-by-chart"] canvas`).then($canvas => { - // click 'other' - cy.wrap($canvas).scrollIntoView(); - cy.wrap($canvas).trigger( - 'mouseover', - drillClickCoordinates[1][0], - drillClickCoordinates[1][1], - ); - cy.wrap($canvas).rightclick( - drillClickCoordinates[1][0], - drillClickCoordinates[1][1], - ); - - drillBy(furtherDrillDimension).then(intercepted => { - verifyExpectedFormData(intercepted, { - groupby: [furtherDrillDimension], - adhoc_filters: [ - { - clause: 'WHERE', - comparator: 'boy', - expressionType: 'SIMPLE', - operator: '==', - operatorId: 'EQUALS', - subject: 'gender', - }, - { - clause: 'WHERE', - comparator: 'other', - expressionType: 'SIMPLE', - operator: '==', - operatorId: 'EQUALS', - subject: 'state', - }, - ], - }); - }); - - cy.get('@drillByModal') - .find('[data-test="drill-by-chart"]') - .should('be.visible'); - - // undo - back to drill by state - interceptV1ChartData('drillByUndo'); - cy.get('@drillByModal') - .find('.ant-breadcrumb') - .should('be.visible') - .and('contain', 'gender (boy)') - .and('contain', '/') - .and('contain', 'state (other)') - .and('contain', furtherDrillDimension) - .contains('state (other)') - .click(); - cy.wait('@drillByUndo').then(intercepted => { - verifyExpectedFormData(intercepted, { - groupby: ['state'], - adhoc_filters: [ - { - clause: 'WHERE', - comparator: 'boy', - expressionType: 'SIMPLE', - operator: '==', - operatorId: 'EQUALS', - subject: 'gender', - }, - ], - }); - }); - - cy.get('@drillByModal') - .find('.ant-breadcrumb') - .should('be.visible') - .and('contain', 'gender (boy)') - .and('contain', '/') - .and('not.contain', 'state (other)') - .and('not.contain', furtherDrillDimension) - .and('contain', 'state'); - - cy.get('@drillByModal') - .find('[data-test="drill-by-chart"]') - .should('be.visible'); - }); - }); -}; - -describe('Drill by modal', () => { - beforeEach(() => { - closeModal(); - }); - before(() => { - interceptDrillInfo(); - cy.visit(SUPPORTED_CHARTS_DASHBOARD); - }); - - describe('Modal actions + Table', () => { - before(() => { - closeModal(); - interceptDrillInfo(); - openTopLevelTab('Tier 1'); - SUPPORTED_TIER1_CHARTS.forEach(waitForChartLoad); - }); - - it.only('opens the modal from the context menu', () => { - openTableContextMenu('boy'); - drillBy('state').then(intercepted => { - verifyExpectedFormData(intercepted, { - groupby: ['state'], - adhoc_filters: [ - { - clause: 'WHERE', - comparator: 'boy', - expressionType: 'SIMPLE', - operator: '==', - operatorId: 'EQUALS', - subject: 'gender', - }, - ], - }); - }); - - cy.getBySel('"Drill by: Table-modal"').as('drillByModal'); - - cy.get('@drillByModal') - .find('.draggable-trigger') - .should('contain', 'Drill by: Table'); - - cy.get('@drillByModal') - .find('[data-test="metadata-bar"]') - .should('be.visible'); - - cy.get('@drillByModal') - .find('.ant-breadcrumb') - .should('be.visible') - .and('contain', 'gender (boy)') - .and('contain', '/') - .and('contain', 'state'); - - cy.get('@drillByModal') - .find('[data-test="drill-by-chart"]') - .should('be.visible') - .and('contain', 'state') - .and('contain', 'sum__num'); - - // further drilling - openTableContextMenu('CA', '[data-test="drill-by-chart"]'); - drillBy('name').then(intercepted => { - verifyExpectedFormData(intercepted, { - groupby: ['name'], - adhoc_filters: [ - { - clause: 'WHERE', - comparator: 'boy', - expressionType: 'SIMPLE', - operator: '==', - operatorId: 'EQUALS', - subject: 'gender', - }, - { - clause: 'WHERE', - comparator: 'CA', - expressionType: 'SIMPLE', - operator: '==', - operatorId: 'EQUALS', - subject: 'state', - }, - ], - }); - }); - - cy.get('@drillByModal') - .find('[data-test="drill-by-chart"]') - .should('be.visible') - .and('not.contain', 'state') - .and('contain', 'name') - .and('contain', 'sum__num'); - - // undo - back to drill by state - interceptV1ChartData('drillByUndo'); - interceptFormDataKey(); - cy.get('@drillByModal') - .find('.ant-breadcrumb') - .should('be.visible') - .and('contain', 'gender (boy)') - .and('contain', '/') - .and('contain', 'state (CA)') - .and('contain', 'name') - .contains('state (CA)') - .click(); - cy.wait('@drillByUndo').then(intercepted => { - verifyExpectedFormData(intercepted, { - groupby: ['state'], - adhoc_filters: [ - { - clause: 'WHERE', - comparator: 'boy', - expressionType: 'SIMPLE', - operator: '==', - operatorId: 'EQUALS', - subject: 'gender', - }, - ], - }); - }); - - cy.get('@drillByModal') - .find('[data-test="drill-by-chart"]') - .should('be.visible') - .and('not.contain', 'name') - .and('contain', 'state') - .and('contain', 'sum__num'); - - cy.get('@drillByModal') - .find('.ant-breadcrumb') - .should('be.visible') - .and('contain', 'gender (boy)') - .and('contain', '/') - .and('not.contain', 'state (CA)') - .and('not.contain', 'name') - .and('contain', 'state'); - - cy.get('@drillByModal') - .find('[data-test="drill-by-display-toggle"]') - .contains('Table') - .click(); - - cy.getBySel('drill-by-chart').should('not.exist'); - - cy.get('@drillByModal') - .find('[data-test="drill-by-results-table"]') - .should('be.visible'); - - cy.wait('@formDataKey').then(intercept => { - cy.get('@drillByModal') - .contains('Edit chart') - .should('have.attr', 'href') - .and( - 'contain', - `/explore/?form_data_key=${intercept.response?.body?.key}`, - ); - }); - }); - }); - - describe('Tier 1 charts', () => { - before(() => { - closeModal(); - interceptDrillInfo(); - openTopLevelTab('Tier 1'); - SUPPORTED_TIER1_CHARTS.forEach(waitForChartLoad); - }); - - it('Pivot Table', () => { - openTableContextMenu('boy', "[data-test-viz-type='pivot_table_v2']"); - drillBy('name').then(intercepted => { - verifyExpectedFormData(intercepted, { - groupbyRows: ['state'], - groupbyColumns: ['name'], - adhoc_filters: [ - { - clause: 'WHERE', - comparator: 'boy', - expressionType: 'SIMPLE', - operator: '==', - operatorId: 'EQUALS', - subject: 'gender', - }, - ], - }); - }); - - cy.getBySel('"Drill by: Pivot Table-modal"').as('drillByModal'); - - cy.get('@drillByModal') - .find('.draggable-trigger') - .should('contain', 'Drill by: Pivot Table'); - - cy.get('@drillByModal') - .find('.ant-breadcrumb') - .should('be.visible') - .and('contain', 'gender (boy)') - .and('contain', '/') - .and('contain', 'name'); - - cy.get('@drillByModal') - .find('[data-test="drill-by-chart"]') - .should('be.visible') - .and('contain', 'state') - .and('contain', 'name') - .and('contain', 'sum__num') - .and('not.contain', 'Gender'); - - openTableContextMenu('CA', '[data-test="drill-by-chart"]'); - drillBy('ds').then(intercepted => { - verifyExpectedFormData(intercepted, { - groupbyColumns: ['name'], - groupbyRows: ['ds'], - adhoc_filters: [ - { - clause: 'WHERE', - comparator: 'boy', - expressionType: 'SIMPLE', - operator: '==', - operatorId: 'EQUALS', - subject: 'gender', - }, - { - clause: 'WHERE', - comparator: 'CA', - expressionType: 'SIMPLE', - operator: '==', - operatorId: 'EQUALS', - subject: 'state', - }, - ], - }); - }); - - cy.get('@drillByModal') - .find('[data-test="drill-by-chart"]') - .should('be.visible') - .and('contain', 'name') - .and('contain', 'ds') - .and('contain', 'sum__num') - .and('not.contain', 'state'); - - interceptV1ChartData('drillByUndo'); - - cy.get('@drillByModal') - .find('.ant-breadcrumb') - .should('be.visible') - .and('contain', 'gender (boy)') - .and('contain', '/') - .and('contain', 'name (CA)') - .and('contain', 'ds') - .contains('name (CA)') - .click(); - cy.wait('@drillByUndo').then(intercepted => { - verifyExpectedFormData(intercepted, { - groupbyRows: ['state'], - groupbyColumns: ['name'], - adhoc_filters: [ - { - clause: 'WHERE', - comparator: 'boy', - expressionType: 'SIMPLE', - operator: '==', - operatorId: 'EQUALS', - subject: 'gender', - }, - ], - }); - }); - - cy.get('@drillByModal') - .find('[data-test="drill-by-chart"]') - .should('be.visible') - .and('not.contain', 'ds') - .and('contain', 'state') - .and('contain', 'name') - .and('contain', 'sum__num'); - - cy.get('@drillByModal') - .find('.ant-breadcrumb') - .should('be.visible') - .and('contain', 'gender (boy)') - .and('contain', '/') - .and('not.contain', 'name (CA)') - .and('not.contain', 'ds') - .and('contain', 'name'); - }); - - it('Line chart', () => { - testEchart('echarts_timeseries_line', 'Line Chart', [ - [85, 93], - [85, 93], - ]); - }); - - it('Area Chart', () => { - testEchart('echarts_area', 'Area Chart', [ - [85, 93], - [85, 93], - ]); - }); - - it('Scatter Chart', () => { - testEchart('echarts_timeseries_scatter', 'Scatter Chart', [ - [85, 93], - [85, 93], - ]); - }); - - it.skip('Bar Chart', () => { - testEchart('echarts_timeseries_bar', 'Bar Chart', [ - [85, 94], - [490, 68], - ]); - }); - - it('Pie Chart', () => { - testEchart('pie', 'Pie Chart', [ - [243, 167], - [534, 248], - ]); - }); - }); - - describe('Tier 2 charts', () => { - before(() => { - closeModal(); - interceptDrillInfo(); - openTopLevelTab('Tier 2'); - SUPPORTED_TIER2_CHARTS.forEach(waitForChartLoad); - }); - - it('Box Plot Chart', () => { - testEchart( - 'box_plot', - 'Box Plot Chart', - [ - [139, 277], - [787, 441], - ], - 'ds', - ); - }); - - it('Generic Chart', () => { - testEchart('echarts_timeseries', 'Generic Chart', [ - [85, 93], - [85, 93], - ]); - }); - - it('Smooth Line Chart', () => { - testEchart('echarts_timeseries_smooth', 'Smooth Line Chart', [ - [85, 93], - [85, 93], - ]); - }); - - it('Step Line Chart', () => { - testEchart('echarts_timeseries_step', 'Step Line Chart', [ - [85, 93], - [85, 93], - ]); - }); - - it('Funnel Chart', () => { - testEchart('funnel', 'Funnel Chart', [ - [154, 80], - [421, 39], - ]); - }); - - it('Gauge Chart', () => { - testEchart('gauge_chart', 'Gauge Chart', [ - [151, 95], - [300, 143], - ]); - }); - - it.skip('Radar Chart', () => { - testEchart('radar', 'Radar Chart', [ - [182, 49], - [423, 91], - ]); - }); - - it('Treemap V2 Chart', () => { - testEchart('treemap_v2', 'Treemap V2 Chart', [ - [145, 84], - [220, 105], - ]); - }); - - it.skip('Mixed Chart', () => { - cy.get('[data-test-viz-type="mixed_timeseries"] canvas').then($canvas => { - // click 'boy' - cy.wrap($canvas).scrollIntoView(); - cy.wrap($canvas).trigger('mouseover', 85, 93); - cy.wrap($canvas).rightclick(85, 93); - - drillBy('name').then(intercepted => { - const { queries } = intercepted.request.body; - expect(queries[0].columns).to.eql(['name']); - expect(queries[0].filters).to.eql([ - { col: 'gender', op: '==', val: 'boy' }, - ]); - expect(queries[1].columns).to.eql(['state']); - expect(queries[1].filters).to.eql([]); - }); - - cy.getBySel('"Drill by: Mixed Chart-modal"').as('drillByModal'); - - cy.get('@drillByModal') - .find('.draggable-trigger') - .should('contain', 'Mixed Chart'); - - cy.get('@drillByModal') - .find('.ant-breadcrumb') - .should('be.visible') - .and('contain', 'gender (boy)') - .and('contain', '/') - .and('contain', 'name'); - - cy.get('@drillByModal') - .find('[data-test="drill-by-chart"]') - .should('be.visible'); - - // further drill - cy.get(`[data-test="drill-by-chart"] canvas`).then($canvas => { - // click second query - cy.wrap($canvas).scrollIntoView(); - cy.wrap($canvas).trigger('mouseover', 261, 114); - cy.wrap($canvas).rightclick(261, 114); - - drillBy('ds').then(intercepted => { - const { queries } = intercepted.request.body; - expect(queries[0].columns).to.eql(['name']); - expect(queries[0].filters).to.eql([ - { col: 'gender', op: '==', val: 'boy' }, - ]); - expect(queries[1].columns).to.eql(['ds']); - expect(queries[1].filters).to.eql([ - { col: 'state', op: '==', val: 'other' }, - ]); - }); - - cy.get('@drillByModal') - .find('[data-test="drill-by-chart"]') - .should('be.visible'); - - // undo - back to drill by state - interceptV1ChartData('drillByUndo'); - cy.get('@drillByModal') - .find('.ant-breadcrumb') - .should('be.visible') - .and('contain', 'gender (boy)') - .and('contain', '/') - .and('contain', 'name (other)') - .and('contain', 'ds') - .contains('name (other)') - .click(); - - cy.wait('@drillByUndo').then(intercepted => { - const { queries } = intercepted.request.body; - expect(queries[0].columns).to.eql(['name']); - expect(queries[0].filters).to.eql([ - { col: 'gender', op: '==', val: 'boy' }, - ]); - expect(queries[1].columns).to.eql(['state']); - expect(queries[1].filters).to.eql([]); - }); - - cy.get('@drillByModal') - .find('.ant-breadcrumb') - .should('be.visible') - .and('contain', 'gender (boy)') - .and('contain', '/') - .and('not.contain', 'name (other)') - .and('not.contain', 'ds') - .and('contain', 'name'); - - cy.get('@drillByModal') - .find('[data-test="drill-by-chart"]') - .should('be.visible'); - }); - }); - }); - }); -}); diff --git a/superset-frontend/package-lock.json b/superset-frontend/package-lock.json index d2f9ca5b3103..207cb69f11e2 100644 --- a/superset-frontend/package-lock.json +++ b/superset-frontend/package-lock.json @@ -191,7 +191,6 @@ "@types/react-resizable": "^3.0.8", "@types/react-router-dom": "^5.3.3", "@types/react-transition-group": "^4.4.12", - "@types/react-ultimate-pagination": "^1.2.4", "@types/react-virtualized-auto-sizer": "^1.0.8", "@types/react-window": "^1.8.8", "@types/redux-localstorage": "^1.0.8", @@ -16078,16 +16077,6 @@ "@types/react": "*" } }, - "node_modules/@types/react-ultimate-pagination": { - "version": "1.2.4", - "resolved": "https://registry.npmjs.org/@types/react-ultimate-pagination/-/react-ultimate-pagination-1.2.4.tgz", - "integrity": "sha512-1y9jLt3KEFGzFD+99qVpJUI/Eu4cEx48sClB957eGoepWRLVVi+r1UBj0157Mg7HYZcIF4I1/qGZYaBBQWhaqg==", - "dev": true, - "license": "MIT", - "dependencies": { - "@types/react": "*" - } - }, "node_modules/@types/react-virtualized-auto-sizer": { "version": "1.0.8", "resolved": "https://registry.npmjs.org/@types/react-virtualized-auto-sizer/-/react-virtualized-auto-sizer-1.0.8.tgz", @@ -60642,7 +60631,7 @@ }, "packages/superset-core": { "name": "@apache-superset/core", - "version": "0.0.1-rc2", + "version": "0.0.1-rc3", "license": "ISC", "devDependencies": { "@babel/cli": "^7.26.4", @@ -60652,7 +60641,8 @@ "@babel/preset-typescript": "^7.26.0", "@types/react": "^17.0.83", "install": "^0.13.0", - "npm": "^11.1.0" + "npm": "^11.1.0", + "typescript": "^5.0.0" }, "peerDependencies": { "antd": "^5.24.6", diff --git a/superset-frontend/package.json b/superset-frontend/package.json index 8ab2eb6408bd..39920371e1ea 100644 --- a/superset-frontend/package.json +++ b/superset-frontend/package.json @@ -259,7 +259,6 @@ "@types/react-resizable": "^3.0.8", "@types/react-router-dom": "^5.3.3", "@types/react-transition-group": "^4.4.12", - "@types/react-ultimate-pagination": "^1.2.4", "@types/react-virtualized-auto-sizer": "^1.0.8", "@types/react-window": "^1.8.8", "@types/redux-localstorage": "^1.0.8", diff --git a/superset-frontend/packages/superset-ui-core/src/components/Pagination/Ellipsis.test.tsx b/superset-frontend/packages/superset-ui-core/src/components/Pagination/Ellipsis.test.tsx deleted file mode 100644 index bb26d7848e81..000000000000 --- a/superset-frontend/packages/superset-ui-core/src/components/Pagination/Ellipsis.test.tsx +++ /dev/null @@ -1,37 +0,0 @@ -/** - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -import { render, screen, userEvent } from '@superset-ui/core/spec'; -import { Ellipsis } from './Ellipsis'; - -test('Ellipsis - click when the button is enabled', async () => { - const click = jest.fn(); - render(); - expect(click).toHaveBeenCalledTimes(0); - await userEvent.click(screen.getByRole('button')); - expect(click).toHaveBeenCalledTimes(1); -}); - -test('Ellipsis - click when the button is disabled', async () => { - const click = jest.fn(); - render(); - expect(click).toHaveBeenCalledTimes(0); - await userEvent.click(screen.getByRole('button')); - expect(click).toHaveBeenCalledTimes(0); -}); diff --git a/superset-frontend/packages/superset-ui-core/src/components/Pagination/Ellipsis.tsx b/superset-frontend/packages/superset-ui-core/src/components/Pagination/Ellipsis.tsx deleted file mode 100644 index 4f1e744cd1aa..000000000000 --- a/superset-frontend/packages/superset-ui-core/src/components/Pagination/Ellipsis.tsx +++ /dev/null @@ -1,38 +0,0 @@ -/** - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -import classNames from 'classnames'; -import { PaginationButtonProps } from './types'; - -export function Ellipsis({ disabled, onClick }: PaginationButtonProps) { - return ( -
  • - { - e.preventDefault(); - if (!disabled) onClick(e); - }} - > - … - -
  • - ); -} diff --git a/superset-frontend/packages/superset-ui-core/src/components/Pagination/Item.test.tsx b/superset-frontend/packages/superset-ui-core/src/components/Pagination/Item.test.tsx deleted file mode 100644 index 9be49cf68c35..000000000000 --- a/superset-frontend/packages/superset-ui-core/src/components/Pagination/Item.test.tsx +++ /dev/null @@ -1,47 +0,0 @@ -/** - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -import { render, screen, userEvent } from '@superset-ui/core/spec'; -import { Item } from './Item'; - -test('Item - click when the item is not active', async () => { - const click = jest.fn(); - render( - -
    - , - ); - expect(click).toHaveBeenCalledTimes(0); - await userEvent.click(screen.getByRole('button')); - expect(click).toHaveBeenCalledTimes(1); - expect(screen.getByTestId('test')).toBeInTheDocument(); -}); - -test('Item - click when the item is active', async () => { - const click = jest.fn(); - render( - -
    - , - ); - expect(click).toHaveBeenCalledTimes(0); - await userEvent.click(screen.getByRole('button')); - expect(click).toHaveBeenCalledTimes(0); - expect(screen.getByTestId('test')).toBeInTheDocument(); -}); diff --git a/superset-frontend/packages/superset-ui-core/src/components/Pagination/Item.tsx b/superset-frontend/packages/superset-ui-core/src/components/Pagination/Item.tsx deleted file mode 100644 index b71da3c93929..000000000000 --- a/superset-frontend/packages/superset-ui-core/src/components/Pagination/Item.tsx +++ /dev/null @@ -1,45 +0,0 @@ -/** - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -import { ReactNode } from 'react'; -import classNames from 'classnames'; -import { PaginationButtonProps } from './types'; - -interface PaginationItemButton extends PaginationButtonProps { - active?: boolean; - children: ReactNode; -} - -export function Item({ active, children, onClick }: PaginationItemButton) { - return ( -
  • - { - e.preventDefault(); - if (!active) onClick(e); - }} - > - {children} - -
  • - ); -} diff --git a/superset-frontend/packages/superset-ui-core/src/components/Pagination/Next.test.tsx b/superset-frontend/packages/superset-ui-core/src/components/Pagination/Next.test.tsx deleted file mode 100644 index 7b17857ff472..000000000000 --- a/superset-frontend/packages/superset-ui-core/src/components/Pagination/Next.test.tsx +++ /dev/null @@ -1,37 +0,0 @@ -/** - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -import { render, screen, userEvent } from '@superset-ui/core/spec'; -import { Next } from './Next'; - -test('Next - click when the button is enabled', async () => { - const click = jest.fn(); - render(); - expect(click).toHaveBeenCalledTimes(0); - await userEvent.click(screen.getByRole('button')); - expect(click).toHaveBeenCalledTimes(1); -}); - -test('Next - click when the button is disabled', async () => { - const click = jest.fn(); - render(); - expect(click).toHaveBeenCalledTimes(0); - await userEvent.click(screen.getByRole('button')); - expect(click).toHaveBeenCalledTimes(0); -}); diff --git a/superset-frontend/packages/superset-ui-core/src/components/Pagination/Next.tsx b/superset-frontend/packages/superset-ui-core/src/components/Pagination/Next.tsx deleted file mode 100644 index 689bc479b61c..000000000000 --- a/superset-frontend/packages/superset-ui-core/src/components/Pagination/Next.tsx +++ /dev/null @@ -1,38 +0,0 @@ -/** - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -import classNames from 'classnames'; -import { PaginationButtonProps } from './types'; - -export function Next({ disabled, onClick }: PaginationButtonProps) { - return ( -
  • - { - e.preventDefault(); - if (!disabled) onClick(e); - }} - > - » - -
  • - ); -} diff --git a/superset-frontend/packages/superset-ui-core/src/components/Pagination/Prev.test.tsx b/superset-frontend/packages/superset-ui-core/src/components/Pagination/Prev.test.tsx deleted file mode 100644 index 576817b37724..000000000000 --- a/superset-frontend/packages/superset-ui-core/src/components/Pagination/Prev.test.tsx +++ /dev/null @@ -1,37 +0,0 @@ -/** - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -import { render, screen, userEvent } from '@superset-ui/core/spec'; -import { Prev } from './Prev'; - -test('Prev - click when the button is enabled', async () => { - const click = jest.fn(); - render(); - expect(click).toHaveBeenCalledTimes(0); - await userEvent.click(screen.getByRole('button')); - expect(click).toHaveBeenCalledTimes(1); -}); - -test('Prev - click when the button is disabled', async () => { - const click = jest.fn(); - render(); - expect(click).toHaveBeenCalledTimes(0); - await userEvent.click(screen.getByRole('button')); - expect(click).toHaveBeenCalledTimes(0); -}); diff --git a/superset-frontend/packages/superset-ui-core/src/components/Pagination/Prev.tsx b/superset-frontend/packages/superset-ui-core/src/components/Pagination/Prev.tsx deleted file mode 100644 index 0de334ae74b8..000000000000 --- a/superset-frontend/packages/superset-ui-core/src/components/Pagination/Prev.tsx +++ /dev/null @@ -1,38 +0,0 @@ -/** - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -import classNames from 'classnames'; -import { PaginationButtonProps } from './types'; - -export function Prev({ disabled, onClick }: PaginationButtonProps) { - return ( -
  • - { - e.preventDefault(); - if (!disabled) onClick(e); - }} - > - « - -
  • - ); -} diff --git a/superset-frontend/packages/superset-ui-core/src/components/Pagination/Wrapper.test.tsx b/superset-frontend/packages/superset-ui-core/src/components/Pagination/Wrapper.test.tsx deleted file mode 100644 index 1dc61e59d771..000000000000 --- a/superset-frontend/packages/superset-ui-core/src/components/Pagination/Wrapper.test.tsx +++ /dev/null @@ -1,75 +0,0 @@ -/** - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -import { render, screen, cleanup } from '@superset-ui/core/spec'; -import Wrapper from './Wrapper'; - -// Add cleanup after each test -afterEach(async () => { - cleanup(); - // Wait for any pending effects to complete - await new Promise(resolve => setTimeout(resolve, 0)); -}); - -jest.mock('./Next', () => ({ - Next: () =>
    , -})); -jest.mock('./Prev', () => ({ - Prev: () =>
    , -})); -jest.mock('./Item', () => ({ - Item: () =>
    , -})); -jest.mock('./Ellipsis', () => ({ - Ellipsis: () =>
    , -})); - -test('Pagination rendering correctly', async () => { - render( - -
  • - , - ); - expect(screen.getByRole('navigation')).toBeInTheDocument(); - expect(screen.getByTestId('test')).toBeInTheDocument(); -}); - -test('Next attribute', async () => { - render(); - expect(screen.getByTestId('next')).toBeInTheDocument(); -}); - -test('Prev attribute', async () => { - render(); - expect(screen.getByTestId('next')).toBeInTheDocument(); -}); - -test('Item attribute', async () => { - render( - - <> - , - ); - expect(screen.getByTestId('item')).toBeInTheDocument(); -}); - -test('Ellipsis attribute', async () => { - render(); - expect(screen.getByTestId('ellipsis')).toBeInTheDocument(); -}); diff --git a/superset-frontend/packages/superset-ui-core/src/components/Pagination/Wrapper.tsx b/superset-frontend/packages/superset-ui-core/src/components/Pagination/Wrapper.tsx deleted file mode 100644 index a7ee66feace9..000000000000 --- a/superset-frontend/packages/superset-ui-core/src/components/Pagination/Wrapper.tsx +++ /dev/null @@ -1,90 +0,0 @@ -/** - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -import { styled } from '@superset-ui/core'; -import { Next } from './Next'; -import { Prev } from './Prev'; -import { Item } from './Item'; -import { Ellipsis } from './Ellipsis'; - -interface PaginationProps { - children: JSX.Element | JSX.Element[]; -} - -const PaginationList = styled.ul` - ${({ theme }) => ` - display: inline-block; - padding: ${theme.sizeUnit * 3}px; - - li { - display: inline; - margin: 0 4px; - - > span { - padding: 8px 12px; - text-decoration: none; - background-color: ${theme.colorBgContainer}; - border: 1px solid ${theme.colorBorder}; - border-radius: ${theme.borderRadius}px; - color: ${theme.colorText}; - - &:hover, - &:focus { - z-index: 2; - color: ${theme.colorText}; - background-color: ${theme.colorBgLayout}; - } - } - - &.disabled { - span { - background-color: transparent; - cursor: default; - - &:focus { - outline: none; - } - } - } - &.active { - span { - z-index: 3; - color: ${theme.colorBgLayout}; - cursor: default; - background-color: ${theme.colorPrimary}; - - &:focus { - outline: none; - } - } - } - } -`} -`; - -function Pagination({ children }: PaginationProps) { - return {children}; -} - -Pagination.Next = Next; -Pagination.Prev = Prev; -Pagination.Item = Item; -Pagination.Ellipsis = Ellipsis; - -export default Pagination; diff --git a/superset-frontend/packages/superset-ui-core/src/components/Pagination/index.tsx b/superset-frontend/packages/superset-ui-core/src/components/Pagination/index.tsx deleted file mode 100644 index c1ba0da0df23..000000000000 --- a/superset-frontend/packages/superset-ui-core/src/components/Pagination/index.tsx +++ /dev/null @@ -1,47 +0,0 @@ -/** - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ -import Pagination from '@superset-ui/core/components/Pagination/Wrapper'; -import { - createUltimatePagination, - ITEM_TYPES, -} from 'react-ultimate-pagination'; - -const ListViewPagination = createUltimatePagination({ - WrapperComponent: Pagination, - itemTypeToComponent: { - [ITEM_TYPES.PAGE]: ({ value, isActive, onClick }) => ( - - {value} - - ), - [ITEM_TYPES.ELLIPSIS]: ({ isActive, onClick }) => ( - - ), - [ITEM_TYPES.PREVIOUS_PAGE_LINK]: ({ isActive, onClick }) => ( - - ), - [ITEM_TYPES.NEXT_PAGE_LINK]: ({ isActive, onClick }) => ( - - ), - [ITEM_TYPES.FIRST_PAGE_LINK]: () => null, - [ITEM_TYPES.LAST_PAGE_LINK]: () => null, - }, -}); - -export default ListViewPagination; diff --git a/superset-frontend/packages/superset-ui-core/src/components/Pagination/types.ts b/superset-frontend/packages/superset-ui-core/src/components/Pagination/types.ts deleted file mode 100644 index 483f88d5cdbd..000000000000 --- a/superset-frontend/packages/superset-ui-core/src/components/Pagination/types.ts +++ /dev/null @@ -1,25 +0,0 @@ -/** - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -import { EventHandler, SyntheticEvent } from 'react'; - -export interface PaginationButtonProps { - disabled?: boolean; - onClick: EventHandler>; -} diff --git a/superset-frontend/packages/superset-ui-core/src/components/TableCollection/TableCollection.test.tsx b/superset-frontend/packages/superset-ui-core/src/components/TableCollection/TableCollection.test.tsx index 52461fff66ac..a99b1d29e799 100644 --- a/superset-frontend/packages/superset-ui-core/src/components/TableCollection/TableCollection.test.tsx +++ b/superset-frontend/packages/superset-ui-core/src/components/TableCollection/TableCollection.test.tsx @@ -100,3 +100,109 @@ test('Should the loading-indicator be visible during loading', () => { expect(screen.getByTestId('loading-indicator')).toBeVisible(); }); + +test('Pagination controls should be rendered when pageSize is provided', () => { + const paginationProps = { + ...defaultProps, + pageSize: 2, + totalCount: 3, + pageIndex: 0, + onPageChange: jest.fn(), + }; + render(); + + expect(screen.getByRole('list')).toBeInTheDocument(); +}); + +test('Pagination should call onPageChange when page is changed', async () => { + const onPageChange = jest.fn(); + const paginationProps = { + ...defaultProps, + pageSize: 2, + totalCount: 3, + pageIndex: 0, + onPageChange, + }; + const { rerender } = render(); + + // Simulate pagination change + await screen.findByTitle('Next Page'); + + // Verify onPageChange would be called with correct arguments + // The actual AntD pagination will handle the click internally + expect(onPageChange).toBeDefined(); + + // Verify that re-rendering with new pageIndex works + rerender(); + expect(screen.getByRole('list')).toBeInTheDocument(); +}); + +test('Pagination callback should be stable across re-renders', () => { + const onPageChange = jest.fn(); + const paginationProps = { + ...defaultProps, + pageSize: 2, + totalCount: 3, + pageIndex: 0, + onPageChange, + }; + + const { rerender } = render(); + + // Re-render with same props + rerender(); + + // onPageChange should not have been called during re-render + expect(onPageChange).not.toHaveBeenCalled(); +}); + +test('Should display correct page info when showRowCount is true', () => { + const paginationProps = { + ...defaultProps, + pageSize: 2, + totalCount: 3, + pageIndex: 0, + onPageChange: jest.fn(), + showRowCount: true, + }; + render(); + + // AntD pagination shows page info + expect(screen.getByText('1-2 of 3')).toBeInTheDocument(); +}); + +test('Should not display page info when showRowCount is false', () => { + const paginationProps = { + ...defaultProps, + pageSize: 2, + totalCount: 3, + pageIndex: 0, + onPageChange: jest.fn(), + showRowCount: false, + }; + render(); + + // Page info should not be shown + expect(screen.queryByText('1-2 of 3')).not.toBeInTheDocument(); +}); + +test('Bulk selection should work with pagination', () => { + const toggleRowSelected = jest.fn(); + const toggleAllRowsSelected = jest.fn(); + const selectionProps = { + ...defaultProps, + bulkSelectEnabled: true, + selectedFlatRows: [], + toggleRowSelected, + toggleAllRowsSelected, + pageSize: 2, + totalCount: 3, + pageIndex: 0, + onPageChange: jest.fn(), + }; + render(); + + // Check that selection checkboxes are rendered + const checkboxes = screen.getAllByRole('checkbox'); + expect(checkboxes.length).toBeGreaterThan(0); +}); diff --git a/superset-frontend/packages/superset-ui-core/src/components/TableCollection/index.tsx b/superset-frontend/packages/superset-ui-core/src/components/TableCollection/index.tsx index 3f18719e689d..9620e323619f 100644 --- a/superset-frontend/packages/superset-ui-core/src/components/TableCollection/index.tsx +++ b/superset-frontend/packages/superset-ui-core/src/components/TableCollection/index.tsx @@ -16,7 +16,7 @@ * specific language governing permissions and limitations * under the License. */ -import { HTMLAttributes, memo, useMemo } from 'react'; +import { HTMLAttributes, memo, useMemo, useCallback } from 'react'; import { ColumnInstance, HeaderGroup, @@ -47,15 +47,25 @@ interface TableCollectionProps { toggleAllRowsSelected?: (value?: boolean) => void; sticky?: boolean; size?: TableSize; + pageIndex?: number; + pageSize?: number; + totalCount?: number; + onPageChange?: (page: number, pageSize: number) => void; + isPaginationSticky?: boolean; + showRowCount?: boolean; } -const StyledTable = styled(Table)` - ${({ theme }) => ` +const StyledTable = styled(Table)<{ + isPaginationSticky?: boolean; + showRowCount?: boolean; +}>` + ${({ theme, isPaginationSticky, showRowCount }) => ` th.ant-column-cell { overflow: hidden; text-overflow: ellipsis; white-space: nowrap; } + .actions { opacity: 0; font-size: ${theme.fontSizeXL}px; @@ -72,15 +82,18 @@ const StyledTable = styled(Table)` } } } + .ant-table-column-title { line-height: initial; } + .ant-table-row:hover { .actions { opacity: 1; transition: opacity ease-in ${theme.motionDurationMid}; } } + .ant-table-cell { max-width: 320px; font-feature-settings: 'tnum' 1; @@ -91,10 +104,37 @@ const StyledTable = styled(Table)` padding-left: ${theme.sizeUnit * 4}px; white-space: nowrap; } + .ant-table-placeholder .ant-table-cell { border-bottom: 0; } + &.ant-table-wrapper .ant-table-pagination.ant-pagination { + display: flex; + justify-content: center; + margin: ${showRowCount ? theme.sizeUnit * 4 : 0}px 0 ${showRowCount ? theme.sizeUnit * 14 : 0}px 0; + position: relative; + + .ant-pagination-total-text { + color: ${theme.colorTextBase}; + margin-inline-end: 0; + position: absolute; + top: ${theme.sizeUnit * 12}px; + } + + ${ + isPaginationSticky && + ` + position: sticky; + bottom: 0; + left: 0; + z-index: 1; + background-color: ${theme.colorBgElevated}; + padding: ${theme.sizeUnit * 2}px 0; + ` + } + } + // Hotfix - antd doesn't apply background color to overflowing cells & table { background-color: ${theme.colorBgContainer}; @@ -116,13 +156,22 @@ function TableCollection({ prepareRow, sticky, size = TableSize.Middle, + pageIndex = 0, + pageSize = 25, + totalCount = 0, + onPageChange, + isPaginationSticky = false, + showRowCount = true, }: TableCollectionProps) { - const mappedColumns = mapColumns( - columns, - headerGroups, - columnsForWrapText, + const mappedColumns = useMemo( + () => mapColumns(columns, headerGroups, columnsForWrapText), + [columns, headerGroups, columnsForWrapText], + ); + + const mappedRows = useMemo( + () => mapRows(rows, prepareRow), + [rows, prepareRow], ); - const mappedRows = mapRows(rows, prepareRow); const selectedRowKeys = useMemo( () => selectedFlatRows?.map(row => row.id) || [], @@ -147,6 +196,68 @@ function TableCollection({ toggleRowSelected, toggleAllRowsSelected, ]); + + const handlePaginationChange = useCallback( + (page: number, size: number) => { + const validPage = Math.max(0, (page || 1) - 1); + const validSize = size || pageSize; + onPageChange?.(validPage, validSize); + }, + [pageSize, onPageChange], + ); + + const showTotalFunc = useCallback( + (total: number, range: [number, number]) => + `${range[0]}-${range[1]} of ${total}`, + [], + ); + + const handleTableChange = useCallback( + (_pagination: any, _filters: any, sorter: SorterResult) => { + if (sorter && sorter.field) { + setSortBy?.([ + { + id: sorter.field, + desc: sorter.order === 'descend', + }, + ] as SortingRule[]); + } + }, + [setSortBy], + ); + + const paginationConfig = useMemo(() => { + if (totalCount === 0) return false; + + const config: any = { + pageSize, + size: 'default' as const, + showSizeChanger: false, + showQuickJumper: false, + align: 'center' as const, + showTotal: showRowCount ? showTotalFunc : undefined, + }; + + if (onPageChange) { + config.current = pageIndex + 1; + config.total = totalCount; + config.onChange = handlePaginationChange; + } else { + if (pageIndex > 0) config.defaultCurrent = pageIndex + 1; + config.total = totalCount; + } + + return config; + }, [ + pageSize, + totalCount, + showRowCount, + showTotalFunc, + pageIndex, + handlePaginationChange, + onPageChange, + ]); + return ( ({ data={mappedRows} size={size} data-test="listview-table" - pagination={false} + pagination={paginationConfig} + scroll={{ x: 'max-content' }} tableLayout="auto" rowKey="rowId" rowSelection={rowSelection} locale={{ emptyText: null }} sortDirections={['ascend', 'descend', 'ascend']} + isPaginationSticky={isPaginationSticky} + showRowCount={showRowCount} components={{ header: { cell: (props: HTMLAttributes) => ( @@ -176,14 +290,7 @@ function TableCollection({ ), }, }} - onChange={(_pagination, _filters, sorter: SorterResult) => { - setSortBy?.([ - { - id: sorter.field, - desc: sorter.order === 'descend', - }, - ] as SortingRule[]); - }} + onChange={handleTableChange} /> ); } diff --git a/superset-frontend/packages/superset-ui-core/src/components/TableView/TableView.test.tsx b/superset-frontend/packages/superset-ui-core/src/components/TableView/TableView.test.tsx index 228ba4dbe344..7d0b9a30005d 100644 --- a/superset-frontend/packages/superset-ui-core/src/components/TableView/TableView.test.tsx +++ b/superset-frontend/packages/superset-ui-core/src/components/TableView/TableView.test.tsx @@ -16,7 +16,7 @@ * specific language governing permissions and limitations * under the License. */ -import { render, screen, userEvent } from '@superset-ui/core/spec'; +import { render, screen, userEvent, waitFor } from '@superset-ui/core/spec'; import { TableView, TableViewProps } from '.'; const mockedProps: TableViewProps = { @@ -30,6 +30,7 @@ const mockedProps: TableViewProps = { { accessor: 'age', Header: 'Age', + sortable: true, id: 'age', }, { @@ -78,10 +79,10 @@ test('should render the cells', () => { test('should render the pagination', () => { render(); - expect(screen.getByRole('navigation')).toBeInTheDocument(); - expect(screen.getAllByRole('button')).toHaveLength(4); - expect(screen.getByText('«')).toBeInTheDocument(); - expect(screen.getByText('»')).toBeInTheDocument(); + expect(screen.getByRole('list')).toBeInTheDocument(); + expect(screen.getAllByRole('button')).toHaveLength(2); + expect(screen.getByTitle('Previous Page')).toBeInTheDocument(); + expect(screen.getByTitle('Next Page')).toBeInTheDocument(); }); test('should show the row count by default', () => { @@ -104,45 +105,63 @@ test('should NOT render the pagination when disabled', () => { withPagination: false, }; render(); - expect(screen.queryByRole('navigation')).not.toBeInTheDocument(); + expect(screen.queryByRole('list')).not.toBeInTheDocument(); }); -test('should NOT render the pagination when fewer rows than page size', () => { +test('should render the pagination even when fewer rows than page size', () => { const withoutPaginationProps = { ...mockedProps, pageSize: 3, }; render(); - expect(screen.queryByRole('navigation')).not.toBeInTheDocument(); + expect(screen.getByRole('list')).toBeInTheDocument(); }); -test('should change page when « and » buttons are clicked', async () => { +test('should change page when pagination is clicked', async () => { render(); - const nextBtn = screen.getByText('»'); - const prevBtn = screen.getByText('«'); - - await userEvent.click(nextBtn); - expect(screen.getAllByRole('cell')).toHaveLength(3); - expect(screen.getByText('321')).toBeInTheDocument(); - expect(screen.getByText('10')).toBeInTheDocument(); - expect(screen.getByText('Kate')).toBeInTheDocument(); - expect(screen.queryByText('Emily')).not.toBeInTheDocument(); - await userEvent.click(prevBtn); - expect(screen.getAllByRole('cell')).toHaveLength(3); expect(screen.getByText('123')).toBeInTheDocument(); expect(screen.getByText('27')).toBeInTheDocument(); expect(screen.getByText('Emily')).toBeInTheDocument(); expect(screen.queryByText('Kate')).not.toBeInTheDocument(); + + const page2 = screen.getByRole('listitem', { name: '2' }); + await userEvent.click(page2); + + await waitFor(() => { + expect(screen.getAllByRole('cell')).toHaveLength(3); + expect(screen.getByText('321')).toBeInTheDocument(); + expect(screen.getByText('10')).toBeInTheDocument(); + expect(screen.getByText('Kate')).toBeInTheDocument(); + expect(screen.queryByText('Emily')).not.toBeInTheDocument(); + }); + + const page1 = screen.getByRole('listitem', { name: '1' }); + await userEvent.click(page1); + + await waitFor(() => { + expect(screen.getAllByRole('cell')).toHaveLength(3); + expect(screen.getByText('123')).toBeInTheDocument(); + expect(screen.getByText('27')).toBeInTheDocument(); + expect(screen.getByText('Emily')).toBeInTheDocument(); + expect(screen.queryByText('Kate')).not.toBeInTheDocument(); + }); }); test('should sort by age', async () => { render(); await userEvent.click(screen.getAllByTestId('sort-header')[1]); - expect(screen.getAllByTestId('table-row-cell')[1]).toHaveTextContent('10'); + + await waitFor(() => { + expect(screen.getAllByTestId('table-row-cell')[1]).toHaveTextContent('10'); + }); + await userEvent.click(screen.getAllByTestId('sort-header')[1]); - expect(screen.getAllByTestId('table-row-cell')[1]).toHaveTextContent('27'); + + await waitFor(() => { + expect(screen.getAllByTestId('table-row-cell')[1]).toHaveTextContent('27'); + }); }); test('should sort by initialSortBy DESC', () => { @@ -208,3 +227,146 @@ test('should render the right wrap content text by columnsForWrapText', () => { 'ant-table-cell-ellipsis', ); }); + +test('should handle server-side pagination', async () => { + const onServerPagination = jest.fn(); + const serverPaginationProps = { + ...mockedProps, + serverPagination: true, + onServerPagination, + totalCount: 10, + pageSize: 2, + }; + render(); + + // Click next page + const page2 = screen.getByRole('listitem', { name: '2' }); + await userEvent.click(page2); + + await waitFor(() => { + expect(onServerPagination).toHaveBeenCalledWith({ + pageIndex: 1, + }); + }); +}); + +test('should handle server-side sorting', async () => { + const onServerPagination = jest.fn(); + const serverPaginationProps = { + ...mockedProps, + serverPagination: true, + onServerPagination, + }; + render(); + + // Click on sortable column + await userEvent.click(screen.getAllByTestId('sort-header')[0]); + + await waitFor(() => { + expect(onServerPagination).toHaveBeenCalledWith({ + pageIndex: 0, + sortBy: [{ id: 'id', desc: false }], + }); + }); +}); + +test('pagination callbacks should be stable across re-renders', () => { + const onServerPagination = jest.fn(); + const serverPaginationProps = { + ...mockedProps, + serverPagination: true, + onServerPagination, + totalCount: 10, + pageSize: 2, + }; + + const { rerender } = render(); + + // Re-render with same props + rerender(); + + // onServerPagination should not have been called during re-render + expect(onServerPagination).not.toHaveBeenCalled(); +}); + +test('should scroll to top when scrollTopOnPagination is true', async () => { + const scrollToSpy = jest + .spyOn(window, 'scrollTo') + .mockImplementation(() => {}); + + const scrollProps = { + ...mockedProps, + scrollTopOnPagination: true, + pageSize: 1, + }; + render(); + + // Click next page + const page2 = screen.getByRole('listitem', { name: '2' }); + await userEvent.click(page2); + + await waitFor(() => { + expect(scrollToSpy).toHaveBeenCalledWith({ top: 0, behavior: 'smooth' }); + }); + + scrollToSpy.mockRestore(); +}); + +test('should NOT scroll to top when scrollTopOnPagination is false', async () => { + const scrollToSpy = jest + .spyOn(window, 'scrollTo') + .mockImplementation(() => {}); + + const scrollProps = { + ...mockedProps, + scrollTopOnPagination: false, + pageSize: 1, + }; + render(); + + // Click next page + const page2 = screen.getByRole('listitem', { name: '2' }); + await userEvent.click(page2); + + await waitFor(() => { + expect(screen.getByText('321')).toBeInTheDocument(); + }); + + expect(scrollToSpy).not.toHaveBeenCalled(); + + scrollToSpy.mockRestore(); +}); + +test('should handle totalCount of 0 correctly', () => { + const emptyProps = { + ...mockedProps, + data: [], + totalCount: 0, + }; + render(); + + // Pagination should not be shown when totalCount is 0 + expect(screen.queryByRole('list')).not.toBeInTheDocument(); +}); + +test('should handle large datasets with pagination', () => { + const largeDataset = Array.from({ length: 100 }, (_, i) => ({ + id: i, + age: 20 + i, + name: `Person ${i}`, + })); + + const largeDataProps = { + ...mockedProps, + data: largeDataset, + pageSize: 10, + }; + render(); + + // Should show only first page (10 items) + expect(screen.getAllByTestId('table-row')).toHaveLength(10); + + // Should show pagination with correct page count + expect(screen.getByRole('list')).toBeInTheDocument(); + expect(screen.getByText('1-10 of 100')).toBeInTheDocument(); +}); diff --git a/superset-frontend/packages/superset-ui-core/src/components/TableView/TableView.tsx b/superset-frontend/packages/superset-ui-core/src/components/TableView/TableView.tsx index ce99de464926..a3d87f7a16f7 100644 --- a/superset-frontend/packages/superset-ui-core/src/components/TableView/TableView.tsx +++ b/superset-frontend/packages/superset-ui-core/src/components/TableView/TableView.tsx @@ -16,16 +16,17 @@ * specific language governing permissions and limitations * under the License. */ -import { memo, useEffect, useRef } from 'react'; +import { memo, useEffect, useRef, useMemo, useCallback } from 'react'; import { isEqual } from 'lodash'; -import { styled, t } from '@superset-ui/core'; +import { styled } from '@superset-ui/core'; import { useFilters, usePagination, useSortBy, useTable } from 'react-table'; import { Empty } from '@superset-ui/core/components'; -import Pagination from '@superset-ui/core/components/Pagination'; import TableCollection from '@superset-ui/core/components/TableCollection'; import { TableSize } from '@superset-ui/core/components/Table'; import { SortByType, ServerPagination } from './types'; +const NOOP_SERVER_PAGINATION = () => {}; + const DEFAULT_PAGE_SIZE = 10; export enum EmptyWrapperType { @@ -96,29 +97,6 @@ const TableViewStyles = styled.div<{ } `; -const PaginationStyles = styled.div<{ - isPaginationSticky?: boolean; -}>` - display: flex; - flex-direction: column; - justify-content: center; - align-items: center; - background-color: ${({ theme }) => theme.colorBgElevated}; - - ${({ isPaginationSticky }) => - isPaginationSticky && - ` - position: sticky; - bottom: 0; - left: 0; - `}; - - .row-count-container { - margin-top: ${({ theme }) => theme.sizeUnit * 2}px; - color: ${({ theme }) => theme.colorText}; - } -`; - const RawTableView = ({ columns, data, @@ -133,16 +111,21 @@ const RawTableView = ({ showRowCount = true, serverPagination = false, columnsForWrapText, - onServerPagination = () => {}, - scrollTopOnPagination = false, + onServerPagination = NOOP_SERVER_PAGINATION, + scrollTopOnPagination = true, size = TableSize.Middle, ...props }: TableViewProps) => { - const initialState = { - pageSize: initialPageSize ?? DEFAULT_PAGE_SIZE, - pageIndex: initialPageIndex ?? 0, - sortBy: initialSortBy, - }; + const tableRef = useRef(null); + + const initialState = useMemo( + () => ({ + pageSize: initialPageSize ?? DEFAULT_PAGE_SIZE, + pageIndex: initialPageIndex ?? 0, + sortBy: initialSortBy, + }), + [initialPageSize, initialPageIndex, initialSortBy], + ); const { getTableProps, @@ -151,10 +134,9 @@ const RawTableView = ({ page, rows, prepareRow, - pageCount, gotoPage, setSortBy, - state: { pageIndex, pageSize, sortBy }, + state: { pageIndex, sortBy }, } = useTable( { columns, @@ -162,36 +144,94 @@ const RawTableView = ({ initialState, manualPagination: serverPagination, manualSortBy: serverPagination, - pageCount: Math.ceil(totalCount / initialState.pageSize), + pageCount: serverPagination + ? Math.ceil(totalCount / initialState.pageSize) + : undefined, + autoResetSortBy: false, }, useFilters, useSortBy, - usePagination, + ...(withPagination ? [usePagination] : []), ); - const content = withPagination ? page : rows; + const EmptyWrapperComponent = useMemo(() => { + switch (emptyWrapperType) { + case EmptyWrapperType.Small: + return ({ children }: any) => <>{children}; + case EmptyWrapperType.Default: + default: + return ({ children }: any) => {children}; + } + }, [emptyWrapperType]); - let EmptyWrapperComponent; - switch (emptyWrapperType) { - case EmptyWrapperType.Small: - EmptyWrapperComponent = ({ children }: any) => <>{children}; - break; - case EmptyWrapperType.Default: - default: - EmptyWrapperComponent = ({ children }: any) => ( - {children} - ); - } + const content = useMemo( + () => (withPagination ? page : rows), + [withPagination, page, rows], + ); - const isEmpty = !loading && content.length === 0; - const hasPagination = pageCount > 1 && withPagination; - const tableRef = useRef(null); - const handleGotoPage = (p: number) => { + const isEmpty = useMemo( + () => !loading && content.length === 0, + [loading, content.length], + ); + + const handleScrollToTop = useCallback(() => { if (scrollTopOnPagination) { - tableRef?.current?.scroll(0, 0); + if (tableRef?.current) { + if (typeof tableRef.current.scrollTo === 'function') { + tableRef.current.scrollTo({ top: 0, behavior: 'smooth' }); + } else if (typeof tableRef.current.scroll === 'function') { + tableRef.current.scroll(0, 0); + } + } + + if (typeof window !== 'undefined' && window.scrollTo) + window.scrollTo({ top: 0, behavior: 'smooth' }); } - gotoPage(p); - }; + }, [scrollTopOnPagination]); + + const handlePageChange = useCallback( + (p: number) => { + if (scrollTopOnPagination) handleScrollToTop(); + + gotoPage(p); + }, + [scrollTopOnPagination, handleScrollToTop, gotoPage], + ); + + const paginationProps = useMemo(() => { + if (!withPagination) { + return { + pageIndex: 0, + pageSize: data.length, + totalCount: 0, + onPageChange: undefined, + }; + } + + if (serverPagination) { + return { + pageIndex, + pageSize: initialPageSize ?? DEFAULT_PAGE_SIZE, + totalCount, + onPageChange: handlePageChange, + }; + } + + return { + pageIndex, + pageSize: initialPageSize ?? DEFAULT_PAGE_SIZE, + totalCount: data.length, + onPageChange: handlePageChange, + }; + }, [ + withPagination, + serverPagination, + pageIndex, + initialPageSize, + totalCount, + data.length, + handlePageChange, + ]); useEffect(() => { if (serverPagination && pageIndex !== initialState.pageIndex) { @@ -199,7 +239,7 @@ const RawTableView = ({ pageIndex, }); } - }, [pageIndex]); + }, [initialState.pageIndex, onServerPagination, pageIndex, serverPagination]); useEffect(() => { if (serverPagination && !isEqual(sortBy, initialState.sortBy)) { @@ -208,61 +248,38 @@ const RawTableView = ({ sortBy, }); } - }, [sortBy]); + }, [initialState.sortBy, onServerPagination, serverPagination, sortBy]); return ( - <> - - - {isEmpty && ( - - {noDataText ? ( - - ) : ( - - )} - - )} - - {hasPagination && ( - - handleGotoPage(p - 1)} - hideFirstAndLastPageLinks - /> - {showRowCount && ( -
    - {!loading && - t( - '%s-%s of %s', - pageSize * pageIndex + (page.length && 1), - pageSize * pageIndex + page.length, - totalCount, - )} -
    + + + {isEmpty && ( + + {noDataText ? ( + + ) : ( + )} -
    + )} - + ); }; diff --git a/superset-frontend/src/components/Chart/DrillBy/DrillByModal.test.tsx b/superset-frontend/src/components/Chart/DrillBy/DrillByModal.test.tsx index 264157c69dbd..3119c16570fe 100644 --- a/superset-frontend/src/components/Chart/DrillBy/DrillByModal.test.tsx +++ b/superset-frontend/src/components/Chart/DrillBy/DrillByModal.test.tsx @@ -356,3 +356,215 @@ describe('Embedded mode behavior', () => { ); }); }); + +describe('Table view with pagination', () => { + beforeEach(() => { + // Mock a large dataset response for pagination testing + const mockLargeDataset = { + result: [ + { + data: Array.from({ length: 100 }, (_, i) => ({ + state: `State${i}`, + sum__num: 1000 + i, + })), + colnames: ['state', 'sum__num'], + coltypes: [1, 0], + }, + ], + }; + + fetchMock.post(CHART_DATA_ENDPOINT, mockLargeDataset, { + overwriteRoutes: true, + }); + }); + + afterEach(() => { + fetchMock.restore(); + }); + + test('should render table view when Table radio is selected', async () => { + await renderModal({ + column: { column_name: 'state', verbose_name: null }, + drillByConfig: { + filters: [{ col: 'gender', op: '==', val: 'boy' }], + groupbyFieldName: 'groupby', + }, + }); + + // Switch to table view + const tableRadio = await screen.findByRole('radio', { name: /table/i }); + userEvent.click(tableRadio); + + // Wait for table to render + 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 () => { + await renderModal({ + column: { column_name: 'state', verbose_name: null }, + drillByConfig: { + filters: [{ col: 'gender', op: '==', val: 'boy' }], + groupbyFieldName: 'groupby', + }, + }); + + // Switch to table view + const tableRadio = await screen.findByRole('radio', { name: /table/i }); + userEvent.click(tableRadio); + + await waitFor(() => { + 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) + await waitFor(() => { + expect(screen.queryByText('State0')).not.toBeInTheDocument(); + }); + }); + + test('should maintain table state when switching between Chart and Table views', async () => { + await renderModal({ + column: { column_name: 'state', verbose_name: null }, + drillByConfig: { + filters: [{ col: 'gender', op: '==', val: 'boy' }], + groupbyFieldName: 'groupby', + }, + }); + + const chartRadio = screen.getByRole('radio', { name: /chart/i }); + const tableRadio = screen.getByRole('radio', { name: /table/i }); + + // Switch to table view + userEvent.click(tableRadio); + await waitFor(() => { + expect(screen.getByTestId('drill-by-results-table')).toBeInTheDocument(); + }); + + // Switch back to chart view + userEvent.click(chartRadio); + await waitFor(() => { + expect(screen.getByTestId('drill-by-chart')).toBeInTheDocument(); + }); + + // Switch back to table view - should maintain state + userEvent.click(tableRadio); + await waitFor(() => { + expect(screen.getByTestId('drill-by-results-table')).toBeInTheDocument(); + }); + }); + + test('should not cause infinite re-renders with pagination', async () => { + // Mock console.error to catch potential infinite loop warnings + const originalError = console.error; + const consoleErrorSpy = jest.fn(); + console.error = consoleErrorSpy; + + await renderModal({ + column: { column_name: 'state', verbose_name: null }, + drillByConfig: { + filters: [{ col: 'gender', op: '==', val: 'boy' }], + groupbyFieldName: 'groupby', + }, + }); + + // Switch to table view + const tableRadio = await screen.findByRole('radio', { name: /table/i }); + userEvent.click(tableRadio); + + await waitFor(() => { + expect(screen.getByTestId('drill-by-results-table')).toBeInTheDocument(); + }); + + // Check that no infinite loop errors were logged + expect(consoleErrorSpy).not.toHaveBeenCalledWith( + expect.stringContaining('Maximum update depth exceeded'), + ); + + console.error = originalError; + }); + + test('should handle empty results in table view', async () => { + // Mock empty dataset response + fetchMock.post( + CHART_DATA_ENDPOINT, + { + result: [ + { + data: [], + colnames: ['state', 'sum__num'], + coltypes: [1, 0], + }, + ], + }, + { overwriteRoutes: true }, + ); + + await renderModal({ + column: { column_name: 'state', verbose_name: null }, + drillByConfig: { + filters: [{ col: 'gender', op: '==', val: 'boy' }], + groupbyFieldName: 'groupby', + }, + }); + + // Switch to table view + const tableRadio = await screen.findByRole('radio', { name: /table/i }); + userEvent.click(tableRadio); + + await waitFor(() => { + expect(screen.getByTestId('drill-by-results-table')).toBeInTheDocument(); + }); + + // Should show empty state + expect(screen.getByText('No data')).toBeInTheDocument(); + }); + + test('should handle sorting in table view', async () => { + await renderModal({ + column: { column_name: 'state', verbose_name: null }, + drillByConfig: { + filters: [{ col: 'gender', op: '==', val: 'boy' }], + groupbyFieldName: 'groupby', + }, + }); + + // Switch to table view + const tableRadio = await screen.findByRole('radio', { name: /table/i }); + userEvent.click(tableRadio); + + await waitFor(() => { + 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(); + }); + }); +}); diff --git a/superset-frontend/src/components/FilterableTable/index.tsx b/superset-frontend/src/components/FilterableTable/index.tsx index e1f98d94d03f..70c939af7ec9 100644 --- a/superset-frontend/src/components/FilterableTable/index.tsx +++ b/superset-frontend/src/components/FilterableTable/index.tsx @@ -123,7 +123,6 @@ export const FilterableTable = ({
    { }); }); - // Update pagination control tests to use button role + // Update pagination control tests for Ant Design pagination it('renders pagination controls', () => { - expect(screen.getByRole('navigation')).toBeInTheDocument(); - expect(screen.getByRole('button', { name: '«' })).toBeInTheDocument(); - expect(screen.getByRole('button', { name: '»' })).toBeInTheDocument(); + const paginationList = screen.getByRole('list'); + expect(paginationList).toBeInTheDocument(); + + const pageOneItem = screen.getByRole('listitem', { name: '1' }); + expect(pageOneItem).toBeInTheDocument(); }); it('calls fetchData on page change', async () => { - const nextButton = screen.getByRole('button', { name: '»' }); - await userEvent.click(nextButton); + const pageTwoItem = screen.getByRole('listitem', { name: '2' }); + await userEvent.click(pageTwoItem); - // Remove sortBy expectation since it's not part of the initial state - expect(mockedProps.fetchData).toHaveBeenCalledWith({ - filters: [], - pageIndex: 1, - pageSize: 1, - sortBy: [], + await waitFor(() => { + const { calls } = mockedProps.fetchData.mock; + const pageChangeCall = calls.find( + call => + call[0].pageIndex === 1 && + call[0].filters.length === 0 && + call[0].pageSize === 1, + ); + expect(pageChangeCall).toBeDefined(); }); }); diff --git a/superset-frontend/src/components/ListView/ListView.tsx b/superset-frontend/src/components/ListView/ListView.tsx index 574327a24763..b156a3e371c0 100644 --- a/superset-frontend/src/components/ListView/ListView.tsx +++ b/superset-frontend/src/components/ListView/ListView.tsx @@ -19,7 +19,6 @@ import { t, styled } from '@superset-ui/core'; import { useCallback, useEffect, useRef, useState, ReactNode } from 'react'; import cx from 'classnames'; -import Pagination from '@superset-ui/core/components/Pagination'; import TableCollection from '@superset-ui/core/components/TableCollection'; import BulkTagModal from 'src/features/tags/BulkTagModal'; import { @@ -71,6 +70,7 @@ const ListViewStyles = styled.div` .body { overflow-x: auto; + overflow-y: hidden; } .ant-empty { @@ -482,6 +482,12 @@ export function ListView({ } }} toggleAllRowsSelected={toggleAllRowsSelected} + pageIndex={pageIndex} + pageSize={pageSize} + totalCount={count} + onPageChange={newPageIndex => { + gotoPage(newPageIndex); + }} /> )} @@ -509,25 +515,6 @@ export function ListView({ )}
  • - {rows.length > 0 && ( -
    - gotoPage(p - 1)} - hideFirstAndLastPageLinks - /> -
    - {!loading && - t( - '%s-%s of %s', - pageSize * pageIndex + (rows.length && 1), - pageSize * pageIndex + rows.length, - count, - )} -
    -
    - )} ); } diff --git a/superset-frontend/src/components/ListView/utils.ts b/superset-frontend/src/components/ListView/utils.ts index 443d57c7d5a8..5329f2fa8979 100644 --- a/superset-frontend/src/components/ListView/utils.ts +++ b/superset-frontend/src/components/ListView/utils.ts @@ -48,15 +48,22 @@ import { // Define custom RisonParam for proper encoding/decoding; note that // %, &, +, and # must be encoded to avoid breaking the url const RisonParam: QueryParamConfig = { - encode: (data?: any | null) => - data === undefined - ? undefined - : rison - .encode(data) - .replace(/%/g, '%25') - .replace(/&/g, '%26') - .replace(/\+/g, '%2B') - .replace(/#/g, '%23'), + encode: (data?: any | null) => { + if (data === undefined || data === null) return undefined; + + const cleanData = JSON.parse( + JSON.stringify(data, (key, value) => + value === undefined ? null : value, + ), + ); + + return rison + .encode(cleanData) + .replace(/%/g, '%25') + .replace(/&/g, '%26') + .replace(/\+/g, '%2B') + .replace(/#/g, '%23'); + }, decode: (dataStr?: string | string[]) => dataStr === undefined || Array.isArray(dataStr) ? undefined @@ -319,7 +326,7 @@ export function useListViewState({ filters: Object.keys(filterObj).length ? filterObj : undefined, pageIndex, }; - if (sortBy[0]) { + if (sortBy?.[0]?.id !== undefined && sortBy[0].id !== null) { queryParams.sortColumn = sortBy[0].id; queryParams.sortOrder = sortBy[0].desc ? 'desc' : 'asc'; } diff --git a/superset-frontend/src/explore/components/DataTablesPane/components/SamplesPane.tsx b/superset-frontend/src/explore/components/DataTablesPane/components/SamplesPane.tsx index 9969dde32e63..c32fc9008610 100644 --- a/superset-frontend/src/explore/components/DataTablesPane/components/SamplesPane.tsx +++ b/superset-frontend/src/explore/components/DataTablesPane/components/SamplesPane.tsx @@ -16,7 +16,7 @@ * specific language governing permissions and limitations * under the License. */ -import { useState, useEffect, useMemo } from 'react'; +import { useState, useEffect, useMemo, useCallback } from 'react'; import { ensureIsArray, GenericDataType, styled, t } from '@superset-ui/core'; import { TableView, @@ -104,6 +104,11 @@ export const SamplesPane = ({ ); const filteredData = useFilteredTableData(filterText, data); + const handleInputChange = useCallback( + (input: string) => setFilterText(input), + [], + ); + if (isLoading) { return ; } @@ -117,7 +122,7 @@ export const SamplesPane = ({ columnTypes={coltypes} rowcount={rowcount} datasourceId={datasourceId} - onInputChange={input => setFilterText(input)} + onInputChange={handleInputChange} isLoading={isLoading} canDownload={canDownload} /> @@ -139,7 +144,7 @@ export const SamplesPane = ({ columnTypes={coltypes} rowcount={rowcount} datasourceId={datasourceId} - onInputChange={input => setFilterText(input)} + onInputChange={handleInputChange} isLoading={isLoading} canDownload={canDownload} /> diff --git a/superset-frontend/src/explore/components/DataTablesPane/components/SingleQueryResultPane.tsx b/superset-frontend/src/explore/components/DataTablesPane/components/SingleQueryResultPane.tsx index 55da157295f0..e2cc2f79c3ac 100644 --- a/superset-frontend/src/explore/components/DataTablesPane/components/SingleQueryResultPane.tsx +++ b/superset-frontend/src/explore/components/DataTablesPane/components/SingleQueryResultPane.tsx @@ -16,7 +16,7 @@ * specific language governing permissions and limitations * under the License. */ -import { useState } from 'react'; +import { useState, useCallback } from 'react'; import { t } from '@superset-ui/core'; import { TableView, @@ -55,6 +55,11 @@ export const SingleQueryResultPane = ({ ); const filteredData = useFilteredTableData(filterText, data); + const handleInputChange = useCallback( + (input: string) => setFilterText(input), + [], + ); + return ( <> setFilterText(input)} + onInputChange={handleInputChange} isLoading={false} canDownload={canDownload} /> diff --git a/superset-frontend/src/explore/components/DataTablesPane/components/useResultsPane.tsx b/superset-frontend/src/explore/components/DataTablesPane/components/useResultsPane.tsx index 625e490388e4..cf7bc096990b 100644 --- a/superset-frontend/src/explore/components/DataTablesPane/components/useResultsPane.tsx +++ b/superset-frontend/src/explore/components/DataTablesPane/components/useResultsPane.tsx @@ -16,7 +16,7 @@ * specific language governing permissions and limitations * under the License. */ -import { useState, useEffect, ReactElement } from 'react'; +import { useState, useEffect, ReactElement, useCallback } from 'react'; import { ensureIsArray, @@ -35,6 +35,14 @@ const Error = styled.pre` margin-top: ${({ theme }) => `${theme.sizeUnit * 4}px`}; `; +const StyledDiv = styled.div` + ${() => ` + display: flex; + height: 100%; + flex-direction: column; + `} +`; + const cache = new WeakMap(); export const useResultsPane = ({ @@ -58,6 +66,8 @@ export const useResultsPane = ({ const queryCount = metadata?.queryObjectCount ?? 1; const isQueryCountDynamic = metadata?.dynamicQueryObjectCount; + const noOpInputChange = useCallback(() => {}, []); + useEffect(() => { // it's an invalid formData when gets a errorMessage if (errorMessage) return; @@ -123,7 +133,7 @@ export const useResultsPane = ({ columnTypes={[]} rowcount={0} datasourceId={queryFormData.datasource} - onInputChange={() => {}} + onInputChange={noOpInputChange} isLoading={false} canDownload={canDownload} /> @@ -144,16 +154,17 @@ export const useResultsPane = ({ : resultResp.slice(0, queryCount); return resultRespToDisplay.map((result, idx) => ( - + + + )); };