Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(component): persist selection across TableNext pages #985

Conversation

chanceaclark
Copy link
Contributor

@chanceaclark chanceaclark commented Sep 26, 2022

What?

Enables TableNext to persist selected items across pagination.

Why?

For each page, we were returning the indexes of items without accounting for pagination. If you were on page 2, the indexes you were getting back were { 0: true, 1: true, 2.0: true, ...etc }. This made it difficult to both Selected all products across pages and persist selected items across pages. This update aligns with our vision of moving toward using @tanstack/react-table or other similar headless table API's to power our TableNext component.

Screenshots/Screen Recordings

Screen.Recording.2022-09-26.at.15.11.54.mov

Testing/Proof

Here's the dev.tsx page I used if you want to test it out yourself:

import { TableNext } from '@bigcommerce/big-design/dist/es/components/TableNext';
import React, { useEffect, useState } from 'react';

interface Item {
  name: string;
  children?: Item[];
}

const items = new Array(25).fill(undefined).map<Item>((_, index) => ({
  name: `Hello ${index}`,
  children: [{ name: `Child ${index}` }, { name: `Child ${index}` }],
}));

const DevPage = () => {
  const [currentPage, onPageChange] = useState(1);
  const [itemsPerPage, onItemsPerPageChange] = useState(10);
  const [selectedItems, onSelectionChange] = useState<Record<string, true>>({});
  const [pagedItems, setPagedItems] = useState<typeof items>([]);
  const [expandedRows, onExpandedChange] = useState({});

  useEffect(() => {
    setPagedItems(
      items.filter(
        (_item, index) =>
          index >= (currentPage - 1) * itemsPerPage && index < currentPage * itemsPerPage,
      ),
    );
  }, [currentPage, itemsPerPage]);

  return (
    <TableNext
      columns={[{ hash: 'name', header: 'Name', render: ({ name }) => name }]}
      expandable={{
        expandedRows,
        onExpandedChange,
        expandedRowSelector: ({ children }) => {
          if (children) {
            return children;
          }

          return undefined;
        },
      }}
      itemName="Worlds"
      items={pagedItems}
      pagination={{
        currentPage,
        onPageChange,
        itemsPerPage,
        totalItems: items.length,
        onItemsPerPageChange,
        itemsPerPageOptions: [10, 20, 30],
      }}
      selectable={{ onSelectionChange, selectedItems }}
    />
  );
};

export default DevPage;
// global.d.ts

// ...

declare module '@bigcommerce/big-design/dist/es/components/TableNext' {
  export * from '@bigcommerce/big-design/dist/components/TableNext';
}

@chanceaclark chanceaclark marked this pull request as ready for review September 26, 2022 20:23
@chanceaclark chanceaclark requested review from a team as code owners September 26, 2022 20:23
@jorgemoya
Copy link
Contributor

Maybe this was discussed before, but shouldn't select 25 select them all? I would've expected all items to be selected across pagination. This still works to persist selection.

Is this just a different ticket issue?

@chanceaclark
Copy link
Contributor Author

Maybe this was discussed before, but shouldn't select 25 select them all? I would've expected all items to be selected across pagination. This still works to persist selection.

Is this just a different ticket issue?

Yeah we've discussed this before. The "Select all" checkbox should just select all the items on the current page. We have another pattern to "Select everything" using bulk actions.

@chanceaclark chanceaclark force-pushed the fix/table-next-pagination-selection branch from 050759e to ebeed64 Compare September 26, 2022 21:15
@chanceaclark chanceaclark force-pushed the fix/table-next-pagination-selection branch from ebeed64 to bc5a85d Compare September 27, 2022 14:17
@chanceaclark chanceaclark force-pushed the fix/table-next-pagination-selection branch from bc5a85d to 6b6fea3 Compare September 27, 2022 14:26
Copy link
Contributor

@jorgemoya jorgemoya left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

@chanceaclark chanceaclark merged commit e80c10a into bigcommerce:master Sep 27, 2022
@chanceaclark chanceaclark deleted the fix/table-next-pagination-selection branch September 27, 2022 15:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants