fix(ListView): implement AntD pagination for ListView component

Author: gabotorresruiz

superset-frontend/packages/superset-ui-core/src/components/TableCollection/index.tsx

@@ -47,6 +47,11 @@ interface TableCollectionProps<T extends object> {
   toggleAllRowsSelected?: (value?: boolean) => void;
   sticky?: boolean;
   size?: TableSize;
+  usePagination?: boolean;
+  pageIndex?: number;
+  pageSize?: number;
+  totalCount?: number;
+  onPageChange?: (page: number, pageSize: number) => void;
 }
 
 const StyledTable = styled(Table)`
@@ -56,6 +61,7 @@ const StyledTable = styled(Table)`
       text-overflow: ellipsis;
       white-space: nowrap;
     }
+
     .actions {
       opacity: 0;
       font-size: ${theme.fontSizeXL}px;
@@ -72,15 +78,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 +100,25 @@ 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: ${theme.sizeUnit * 4}px 0 ${theme.sizeUnit * 14}px 0;
+      position: relative;
+
+      .ant-pagination-total-text {
+        color: ${theme.colorTextBase};
+        margin-inline-end: 0;
+        position: absolute;
+        top: ${theme.sizeUnit * 12}px;
+      }
+    }
+
     // Hotfix - antd doesn't apply background color to overflowing cells
     & table {
       background-color: ${theme.colorBgContainer};
@@ -116,6 +140,11 @@ function TableCollection<T extends object>({
   prepareRow,
   sticky,
   size = TableSize.Middle,
+  usePagination = false,
+  pageIndex = 0,
+  pageSize = 25,
+  totalCount = 0,
+  onPageChange,
 }: TableCollectionProps<T>) {
   const mappedColumns = mapColumns<T>(
     columns,
@@ -147,6 +176,27 @@ function TableCollection<T extends object>({
     toggleRowSelected,
     toggleAllRowsSelected,
   ]);
+
+  const paginationConfig = useMemo(() => {
+    if (!usePagination) return false;
+
+    return {
+      current: pageIndex + 1,
+      pageSize,
+      total: totalCount,
+      size: 'default' as const,
+      showSizeChanger: false,
+      showQuickJumper: false,
+      align: 'center' as const,
+      showTotal: (total: number, range: [number, number]) =>
+        `${range[0]}-${range[1]} of ${total}`,
+      onChange: (page: number, size: number) => {
+        const validPage = Math.max(0, (page || 1) - 1);
+        const validSize = size || pageSize;
+        onPageChange?.(validPage, validSize);
+      },
+    };
+  }, [usePagination, pageIndex, pageSize, totalCount, onPageChange]);
   return (
     <StyledTable
       loading={loading}
@@ -155,7 +205,7 @@ function TableCollection<T extends object>({
       data={mappedRows}
       size={size}
       data-test="listview-table"
-      pagination={false}
+      pagination={paginationConfig}
       tableLayout="auto"
       rowKey="rowId"
       rowSelection={rowSelection}

superset-frontend/src/components/ListView/ListView.test.jsx

@@ -16,7 +16,7 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-import { render, screen, within } from 'spec/helpers/testing-library';
+import { render, screen, within, waitFor } from 'spec/helpers/testing-library';
 import userEvent from '@testing-library/user-event';
 import { QueryParamProvider } from 'use-query-params';
 import thunk from 'redux-thunk';
@@ -172,23 +172,28 @@ describe('ListView', () => {
     });
   });
 
-  // 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();
     });
   });
 

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 {
@@ -462,6 +462,7 @@ export function ListView<T extends object = any>({
                 </FullPageLoadingWrapper>
               ) : (
                 <TableCollection
+                  usePagination
                   getTableProps={getTableProps}
                   getTableBodyProps={getTableBodyProps}
                   prepareRow={prepareRow}
@@ -482,6 +483,12 @@ export function ListView<T extends object = any>({
                     }
                   }}
                   toggleAllRowsSelected={toggleAllRowsSelected}
+                  pageIndex={pageIndex}
+                  pageSize={pageSize}
+                  totalCount={count}
+                  onPageChange={newPageIndex => {
+                    gotoPage(newPageIndex);
+                  }}
                 />
               )}
             </>
@@ -509,25 +516,6 @@ export function ListView<T extends object = any>({
           )}
         </div>
       </div>
-      {rows.length > 0 && (
-        <div className="pagination-container">
-          <Pagination
-            totalPages={pageCount || 0}
-            currentPage={pageCount && pageIndex < pageCount ? pageIndex + 1 : 0}
-            onChange={(p: number) => gotoPage(p - 1)}
-            hideFirstAndLastPageLinks
-          />
-          <div className="row-count-container">
-            {!loading &&
-              t(
-                '%s-%s of %s',
-                pageSize * pageIndex + (rows.length && 1),
-                pageSize * pageIndex + rows.length,
-                count,
-              )}
-          </div>
-        </div>
-      )}
     </ListViewStyles>
   );
 }

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<string, any> = {
-  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';
     }