Merge pull request #1938 from dxc-technology/Mil4n0r/resultsettable_checkbox-fix

Jialecl · web-flow · commit f319e71b5c04 · 2024-05-09T12:57:17.000+02:00
Added new logic for resultsetTable to manage sorting and adding/removing rows properly
diff --git a/lib/src/divider/Divider.stories.tsx b/lib/src/divider/Divider.stories.tsx
@@ -1,7 +1,8 @@
 import React from "react";
 import Title from "../../.storybook/components/Title";
 import DxcDivider from "./Divider";
-import { DxcFlex, DxcParagraph } from "../main";
+import DxcFlex from "../flex/Flex";
+import DxcParagraph from "../paragraph/Paragraph";
 import ExampleContainer from "../../.storybook/components/ExampleContainer";
 
 export default {
diff --git a/lib/src/icon/Icon.stories.tsx b/lib/src/icon/Icon.stories.tsx
@@ -2,7 +2,7 @@ import React from "react";
 import DxcIcon from "./Icon";
 import Title from "../../.storybook/components/Title";
 import ExampleContainer from "../../.storybook/components/ExampleContainer";
-import { DxcTypography } from "../main";
+import DxcTypography from "../typography/Typography";
 
 export default {
   title: "Icon",
diff --git a/lib/src/resultset-table/ResultsetTable.stories.tsx b/lib/src/resultset-table/ResultsetTable.stories.tsx
@@ -409,5 +409,5 @@ export const DropdownAction = ResultsetActionsCellDropdown.bind({});
 DropdownAction.play = async ({ canvasElement }) => {
   const canvas = within(canvasElement);
   const dropdown = canvas.getAllByRole("button")[5];
-  await userEvent.click(dropdown);
+  userEvent.click(dropdown);
 };
diff --git a/lib/src/resultset-table/ResultsetTable.test.tsx b/lib/src/resultset-table/ResultsetTable.test.tsx
@@ -3,6 +3,7 @@ import { render, fireEvent, act } from "@testing-library/react";
 import userEvent from "@testing-library/user-event";
 import DxcResultsetTable from "./ResultsetTable";
 import { ActionCellsPropsType } from "../table/types";
+import DxcCheckbox from "../checkbox/Checkbox";
 
 // Mocking DOMRect for Radix Primitive Popover
 (global as any).globalThis = global;
@@ -178,54 +179,43 @@ const rows = [
     },
   ],
 ];
-const rows2 = [
+
+const columnsWithCheckbox = [
+  { displayValue: "Id", isSortable: true },
+  { displayValue: "Checkbox", isSortable: false },
+  { displayValue: "Name", isSortable: false },
+  { displayValue: "City", isSortable: false },
+];
+
+const rowsWithCheckbox = [
   [
+    { displayValue: "001", sortValue: "001" },
     {
-      displayValue: "546",
-      sortValue: "465",
-    },
-    {
-      displayValue: "OtherValue",
-      sortValue: "OtherValue",
-    },
-    {
-      displayValue: "OtherValue",
-      sortValue: "OtherValue",
+      displayValue: <DxcCheckbox size="fillParent" defaultChecked={true} />,
     },
+    { displayValue: "Peter" },
+    { displayValue: "Miami" },
   ],
   [
+    { displayValue: "002", sortValue: "002" },
     {
-      displayValue: "978",
-      sortValue: "465",
-    },
-    {
-      displayValue: "OtherValue",
-      sortValue: "OtherValue",
-    },
-    {
-      displayValue: "OtherValue",
-      sortValue: "OtherValue",
-    },
-    {
-      displayValue: "",
+      displayValue: <DxcCheckbox size="fillParent" />,
     },
+    { displayValue: "Louis" },
+    { displayValue: "London" },
   ],
   [
+    { displayValue: "003", sortValue: "003" },
     {
-      displayValue: "678",
-      sortValue: "344",
-    },
-    {
-      displayValue: "OtherValue",
-      sortValue: "OtherValue",
-    },
-    {
-      displayValue: "OtherValue",
-      sortValue: "OtherValue",
+      displayValue: <DxcCheckbox size="fillParent" />,
     },
+    { displayValue: "Lana" },
+    { displayValue: "Amsterdam" },
   ],
 ];
 
+const rows2 = [...rows].slice(0, -1);
+
 describe("Resultset table component tests", () => {
   test("Resultset table rendered correctly", () => {
     const { getByText } = render(<DxcResultsetTable columns={columns} rows={rows} itemsPerPage={3} />);
@@ -256,7 +246,7 @@ describe("Resultset table component tests", () => {
     window.HTMLElement.prototype.scrollIntoView = () => {};
     window.HTMLElement.prototype.scrollTo = () => {};
     const { getByText, getAllByRole } = render(
-      <DxcResultsetTable columns={columns} showGoToPage rows={rows} itemsPerPage={3} />
+      <DxcResultsetTable columns={columns} showGoToPage rows={rows} itemsPerPage={3} />,
     );
     expect(getByText("Peter")).toBeTruthy();
     expect(getByText("Louis")).toBeTruthy();
@@ -294,16 +284,57 @@ describe("Resultset table component tests", () => {
     expect(component.queryByText("Cosmin")).not.toBeTruthy();
   });
 
-  test("Resultset table change rows should go to first page", () => {
-    const { queryByText, rerender } = render(<DxcResultsetTable columns={columns} rows={rows} itemsPerPage={3} />);
+  test("Resultset table should go one page back when removing the last page data", () => {
+    const { getAllByRole, queryByText, rerender } = render(
+      <DxcResultsetTable columns={columns} rows={rows} itemsPerPage={3} />,
+    );
+    expect(queryByText("1 to 3 of 10")).toBeTruthy();
+    const lastButton = getAllByRole("button")[4];
     expect(queryByText("Peter")).toBeTruthy();
+    fireEvent.click(lastButton);
+    expect(queryByText("10 to 10 of 10")).toBeTruthy();
     rerender(<DxcResultsetTable columns={columns} rows={rows2} itemsPerPage={3} />);
-    expect(queryByText("1 to 3 of 3")).toBeTruthy();
+    expect(queryByText("7 to 9 of 9")).toBeTruthy();
+  });
+
+  test("Resultset table shouldn't go one page back when there is data left in the last page", () => {
+    const { getAllByRole, queryByText, rerender } = render(
+      <DxcResultsetTable columns={columns} rows={rows} itemsPerPage={2} />,
+    );
+    expect(queryByText("1 to 2 of 10")).toBeTruthy();
+    const lastButton = getAllByRole("button")[4];
+    expect(queryByText("Peter")).toBeTruthy();
+    fireEvent.click(lastButton);
+    expect(queryByText("9 to 10 of 10")).toBeTruthy();
+    rerender(<DxcResultsetTable columns={columns} rows={rows2} itemsPerPage={2} />);
+    expect(queryByText("9 to 9 of 9")).toBeTruthy();
+  });
+
+  test("Resultset table uncontrolled components maintain its value when sorting", async () => {
+    const { getAllByRole } = render(
+      <DxcResultsetTable columns={columnsWithCheckbox} rows={rowsWithCheckbox} itemsPerPage={3} />,
+    );
+    const columnHeader = getAllByRole("columnheader")[0];
+    const sortButton = getAllByRole("button")[0];
+
+    expect(getAllByRole("checkbox")[0].getAttribute("aria-checked")).toBe("true");
+
+    expect(columnHeader.getAttribute("aria-sort")).toBe("none");
+
+    fireEvent.click(sortButton);
+
+    expect(columnHeader.getAttribute("aria-sort")).toBe("ascending");
+
+    fireEvent.click(sortButton);
+
+    expect(columnHeader.getAttribute("aria-sort")).toBe("descending");
+    
+    expect(getAllByRole("checkbox")[0].getAttribute("aria-checked")).toBe("false");
   });
 
   test("Resultset table change itemsPerPage should go to first page", () => {
     const { getAllByRole } = render(
-      <DxcResultsetTable columns={columns} rows={rows} itemsPerPage={3} itemsPerPageOptions={[2, 3]} />
+      <DxcResultsetTable columns={columns} rows={rows} itemsPerPage={3} itemsPerPageOptions={[2, 3]} />,
     );
     const lastButton = getAllByRole("button")[4];
     expect(getAllByRole("row").length - 1).toEqual(3);
@@ -320,7 +351,7 @@ describe("Resultset table component tests", () => {
   test("Resultset table with ActionsCell", () => {
     const onSelectOption = jest.fn();
     const onClick = jest.fn();
-    const actions: ActionCellsPropsType['actions'] = [
+    const actions: ActionCellsPropsType["actions"] = [
       {
         title: "icon1",
         onClick: onSelectOption,
@@ -344,7 +375,7 @@ describe("Resultset table component tests", () => {
         title: "icon2",
         onClick: onClick,
       },
-    ] ;
+    ];
     const actionRows = [
       [
         {
@@ -362,7 +393,7 @@ describe("Resultset table component tests", () => {
       ],
     ];
     const { getAllByRole, getByRole, getByText } = render(
-      <DxcResultsetTable columns={columns} rows={actionRows} itemsPerPage={3} />
+      <DxcResultsetTable columns={columns} rows={actionRows} itemsPerPage={3} />,
     );
     const dropdown = getAllByRole("button")[2];
     act(() => {
diff --git a/lib/src/resultset-table/ResultsetTable.tsx b/lib/src/resultset-table/ResultsetTable.tsx
@@ -1,4 +1,4 @@
-import React, { useState, useMemo, useEffect } from "react";
+import React, { useState, useMemo, useEffect, useRef } from "react";
 import styled, { ThemeProvider } from "styled-components";
 import { spaces } from "../common/variables";
 import DxcPaginator from "../paginator/Paginator";
@@ -12,10 +12,10 @@ import CoreTokens from "../common/coreTokens";
 const normalizeSortValue = (sortValue: string | React.ReactNode) =>
   typeof sortValue === "string" ? sortValue.toUpperCase() : sortValue;
 
-const sortArray = (index: number, order: "ascending" | "descending", resultset: Row[][]) =>
+const sortArray = (index: number, order: "ascending" | "descending", resultset: { id: string; cells: Row[] }[]) =>
   resultset.slice().sort((element1, element2) => {
-    const sortValueA = normalizeSortValue(element1[index].sortValue || element1[index].displayValue);
-    const sortValueB = normalizeSortValue(element2[index].sortValue || element2[index].displayValue);
+    const sortValueA = normalizeSortValue(element1.cells[index].sortValue || element1[index].displayValue);
+    const sortValueB = normalizeSortValue(element2.cells[index].sortValue || element2[index].displayValue);
     let comparison = 0;
     if (typeof sortValueA === "object") {
       comparison = -1;
@@ -36,9 +36,19 @@ const getMaxItemsPerPageIndex = (
   minItemsPerPageIndex: number,
   itemsPerPage: number,
   resultset: Row[][],
-  page: number
+  page: number,
 ) => (minItemsPerPageIndex + itemsPerPage > resultset.length ? resultset.length : itemsPerPage * page - 1);
 
+const assignIdsToRows = (resultset: Row[][]) => {
+  if (resultset.length > 0) {
+    return resultset.map((row, index) => ({
+      cells: row,
+      id: `row_${index}`,
+    }));
+  }
+  return [];
+};
+
 const DxcResultsetTable = ({
   columns,
   rows,
@@ -56,18 +66,23 @@ const DxcResultsetTable = ({
   const [sortColumnIndex, changeSortColumnIndex] = useState(-1);
   const [sortOrder, changeSortOrder] = useState<"ascending" | "descending">("ascending");
 
+  const prevRowCountRef = useRef<number>(rows.length);
+
+  const rowsWithIds = useMemo(() => assignIdsToRows(rows), [rows]);
+
   const minItemsPerPageIndex = useMemo(() => getMinItemsPerPageIndex(page, itemsPerPage, page), [itemsPerPage, page]);
   const maxItemsPerPageIndex = useMemo(
     () => getMaxItemsPerPageIndex(minItemsPerPageIndex, itemsPerPage, rows, page),
-    [itemsPerPage, minItemsPerPageIndex, page, rows]
+    [itemsPerPage, minItemsPerPageIndex, page, rows],
   );
+
   const sortedResultset = useMemo(
-    () => (sortColumnIndex !== -1 ? sortArray(sortColumnIndex, sortOrder, rows) : rows),
-    [sortColumnIndex, sortOrder, rows]
+    () => (sortColumnIndex !== -1 ? sortArray(sortColumnIndex, sortOrder, rowsWithIds) : rowsWithIds),
+    [sortColumnIndex, sortOrder, rowsWithIds],
   );
   const filteredResultset = useMemo(
     () => sortedResultset && sortedResultset.slice(minItemsPerPageIndex, maxItemsPerPageIndex + 1),
-    [sortedResultset, minItemsPerPageIndex, maxItemsPerPageIndex]
+    [sortedResultset, minItemsPerPageIndex, maxItemsPerPageIndex],
   );
 
   const goToPage = (newPage: number) => {
@@ -81,16 +96,25 @@ const DxcResultsetTable = ({
       sortColumnIndex === -1 || sortColumnIndex !== columnIndex
         ? "ascending"
         : sortOrder === "ascending"
-        ? "descending"
-        : "ascending"
+          ? "descending"
+          : "ascending",
     );
   };
 
   useEffect(() => {
     if (!hidePaginator) {
-      rows.length > 0 ? changePage(1) : changePage(0);
+      if (rows.length === 0) {
+        changePage(0);
+      } else if (rows.length < prevRowCountRef.current) {
+        const lastPage = Math.ceil(rows.length / itemsPerPage);
+        const prevLastPage = Math.ceil(prevRowCountRef.current / itemsPerPage);
+        if (lastPage < prevLastPage) {
+          changePage(Math.min(lastPage, page));
+        }
+      }
+      prevRowCountRef.current = rows.length;
     }
-  }, [rows]);
+  }, [rows.length]);
 
   return (
     <ThemeProvider theme={colorsTheme.table}>
@@ -130,9 +154,9 @@ const DxcResultsetTable = ({
             </tr>
           </thead>
           <tbody>
-            {filteredResultset.map((cells, rowIndex) => (
-              <tr key={`resultSetTableCell_${page}_${rowIndex}`}>
-                {cells.map((cellContent, cellIndex) => (
+            {filteredResultset.map((row) => (
+              <tr key={`resultSetTableCell_${row.id}`}>
+                {row.cells.map((cellContent, cellIndex) => (
                   <td key={`resultSetTableCellContent_${cellIndex}`}>{cellContent.displayValue}</td>
                 ))}
               </tr>
@@ -156,7 +180,8 @@ const DxcResultsetTable = ({
   );
 };
 
-const calculateWidth = (margin: string | object) => `calc(100% - ${getMargin(margin, "left")} - ${getMargin(margin, "right")})`;
+const calculateWidth = (margin: string | object) =>
+  `calc(100% - ${getMargin(margin, "left")} - ${getMargin(margin, "right")})`;
 
 const DxcResultsetTableContainer = styled.div<{ margin: ResultsetTablePropsType["margin"] }>`
   width: ${(props) => calculateWidth(props.margin)};
@@ -178,8 +203,8 @@ const HeaderContainer = styled.span<{ isSortable: Column["isSortable"]; mode: Re
     props.theme.headerTextAlign === "center"
       ? "center"
       : props.theme.headerTextAlign === "right"
-      ? "flex-end"
-      : "flex-start"};
+        ? "flex-end"
+        : "flex-start"};
   gap: ${CoreTokens.spacing_8};
   width: fit-content;
   border: 1px solid transparent;
