From 5e7f3de5f306b842fa31cfc9731cc2a63f79b708 Mon Sep 17 00:00:00 2001
From: Jessie Wei <jeswei@amazon.com>
Date: Tue, 19 Jul 2022 12:45:37 +1000
Subject: [PATCH] fix(Table): Fix an issue where select does not work properly
 when groupby is enabled. Close #680 (#684)

* fix(Table): Fix an issue where select does not work properly when groupby is enabled

* chore: Fix lint warning
---
 src/components/Table/Table.md                 |  2 +-
 .../Table/data/columnDefinitions.ts           |  2 +-
 src/components/Table/data/expandable.ts       | 87 +++++++++++++++++++
 src/components/Table/data/groupBy.ts          | 21 -----
 .../hooks/useTableColumnFilter/index.tsx      | 24 ++---
 src/components/Table/index.stories.tsx        | 63 ++++++++++++--
 6 files changed, 159 insertions(+), 40 deletions(-)
 create mode 100644 src/components/Table/data/expandable.ts

diff --git a/src/components/Table/Table.md b/src/components/Table/Table.md
index 8983a78e..b55c1699 100644
--- a/src/components/Table/Table.md
+++ b/src/components/Table/Table.md
@@ -29,7 +29,7 @@ const columnDefinition: Column<DataType>[] = [
 
 ### Examples
 
-**More examples** are available on <a href="https://storybook.northstar.aws-prototyping.cloud/Table?path=/story/table--default" target="_blank">NorthStar Storybook</a>.
+**More examples** are available on <a href="https://storybook.northstar.aws-prototyping.cloud/?path=/docs/components-table--default" target="_blank">NorthStar Storybook</a>.
 
 ```jsx
 import Table from 'aws-northstar/components/Table';
diff --git a/src/components/Table/data/columnDefinitions.ts b/src/components/Table/data/columnDefinitions.ts
index 9a242dc5..f092214b 100644
--- a/src/components/Table/data/columnDefinitions.ts
+++ b/src/components/Table/data/columnDefinitions.ts
@@ -25,7 +25,7 @@ const columnDefinition: Column<DataType>[] = [
     },
     {
         id: 'name',
-        width: 200,
+        width: 120,
         Header: 'Name',
         accessor: 'name',
     },
diff --git a/src/components/Table/data/expandable.ts b/src/components/Table/data/expandable.ts
new file mode 100644
index 00000000..ecec73c8
--- /dev/null
+++ b/src/components/Table/data/expandable.ts
@@ -0,0 +1,87 @@
+/** *******************************************************************************************************************
+  Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
+  
+  Licensed 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 { DataType } from './type';
+
+const data: DataType[] = [
+    {
+        id: 'id0000011',
+        name: 'Order 11',
+        createdDate: '2020-01-01',
+        subRows: [
+            {
+                id: 'id0000111',
+                name: 'Order 111',
+                createdDate: '2020-01-01',
+            },
+        ],
+    },
+    {
+        id: 'id0000012',
+        name: 'Order 12',
+        createdDate: '2020-01-01',
+        subRows: [
+            {
+                id: 'id0000211',
+                name: 'Order 211',
+                createdDate: '2020-01-01',
+            },
+        ],
+    },
+    {
+        id: 'id0000013',
+        name: 'Order 12',
+        createdDate: '2020-01-02',
+        subRows: [
+            {
+                id: 'id0000311',
+                name: 'Order 311',
+                createdDate: '2020-01-01',
+            },
+        ],
+    },
+    {
+        id: 'id0000014',
+        name: 'Order 12',
+        createdDate: '2020-01-02',
+    },
+    {
+        id: 'id0000015',
+        name: 'Order 13',
+        createdDate: '2020-01-02',
+    },
+    {
+        id: 'id0000016',
+        name: 'Order 13',
+        createdDate: '2020-01-03',
+    },
+    {
+        id: 'id0000017',
+        name: 'Order 13',
+        createdDate: '2020-01-03',
+    },
+    {
+        id: 'id0000018',
+        name: 'Order 13',
+        createdDate: '2020-01-03',
+    },
+    {
+        id: 'id0000019',
+        name: 'Order 13',
+        createdDate: '2020-01-04',
+    },
+];
+
+export default data;
diff --git a/src/components/Table/data/groupBy.ts b/src/components/Table/data/groupBy.ts
index ecec73c8..988e6401 100644
--- a/src/components/Table/data/groupBy.ts
+++ b/src/components/Table/data/groupBy.ts
@@ -20,37 +20,16 @@ const data: DataType[] = [
         id: 'id0000011',
         name: 'Order 11',
         createdDate: '2020-01-01',
-        subRows: [
-            {
-                id: 'id0000111',
-                name: 'Order 111',
-                createdDate: '2020-01-01',
-            },
-        ],
     },
     {
         id: 'id0000012',
         name: 'Order 12',
         createdDate: '2020-01-01',
-        subRows: [
-            {
-                id: 'id0000211',
-                name: 'Order 211',
-                createdDate: '2020-01-01',
-            },
-        ],
     },
     {
         id: 'id0000013',
         name: 'Order 12',
         createdDate: '2020-01-02',
-        subRows: [
-            {
-                id: 'id0000311',
-                name: 'Order 311',
-                createdDate: '2020-01-01',
-            },
-        ],
     },
     {
         id: 'id0000014',
diff --git a/src/components/Table/hooks/useTableColumnFilter/index.tsx b/src/components/Table/hooks/useTableColumnFilter/index.tsx
index 283cbb84..1d50938e 100644
--- a/src/components/Table/hooks/useTableColumnFilter/index.tsx
+++ b/src/components/Table/hooks/useTableColumnFilter/index.tsx
@@ -86,17 +86,19 @@ const useTableColumnFilter = <D extends object>({
                                     controlId={row.id}
                                 />
                             ) : (
-                                <RadioButton
-                                    name="select"
-                                    checked={row.isSelected}
-                                    controlId={row.id}
-                                    data-testid={row.id}
-                                    disabled={isSelectDisabled}
-                                    onChange={() => {
-                                        toggleAllRowsSelected(false);
-                                        row.toggleRowSelected(true);
-                                    }}
-                                />
+                                !row.isGrouped && (
+                                    <RadioButton
+                                        name="select"
+                                        checked={row.isSelected}
+                                        controlId={row.id}
+                                        data-testid={row.id}
+                                        disabled={isSelectDisabled}
+                                        onChange={() => {
+                                            toggleAllRowsSelected(false);
+                                            row.toggleRowSelected(true);
+                                        }}
+                                    />
+                                )
                             )}
                         </div>
                     );
diff --git a/src/components/Table/index.stories.tsx b/src/components/Table/index.stories.tsx
index ae2ebc76..9047a819 100644
--- a/src/components/Table/index.stories.tsx
+++ b/src/components/Table/index.stories.tsx
@@ -21,6 +21,7 @@ import Button from '../Button';
 import longData from './data/long';
 import shortData from './data/short';
 import groupByData from './data/groupBy';
+import expandableData from './data/expandable';
 import { DataType } from './data/type';
 import columnDefinitions from './data/columnDefinitions';
 import orderBy from 'lodash.orderby';
@@ -31,6 +32,10 @@ export default {
     title: 'Components/Table',
 };
 
+export const Default = () => (
+    <Table tableTitle={'Default Table'} columnDefinitions={columnDefinitions} items={shortData} />
+);
+
 export const Loading = () => (
     <Table
         tableTitle={'Table'}
@@ -59,15 +64,14 @@ export const Simple = () => (
     />
 );
 
-export const Default = () => (
-    <Table tableTitle={'Default Table'} columnDefinitions={columnDefinitions} items={shortData} />
-);
-
 export const MultiSelect = () => {
     const getRowId = useCallback((data) => data.id, []);
     return (
         <Table
             tableTitle={'Multi Select Table'}
+            tableDescription={
+                'Specifies getRowId to avoid the infinite re-rendering loop when onSelectionChange is used'
+            }
             columnDefinitions={columnDefinitions}
             items={shortData}
             selectedRowIds={['id0000012', 'id0000013']}
@@ -84,6 +88,9 @@ export const MultiSelectWithRowsDisabled = () => {
     return (
         <Table
             tableTitle={'Multi Select Table'}
+            tableDescription={
+                'Specifies getRowId to avoid the infinite re-rendering loop when onSelectionChange is used'
+            }
             columnDefinitions={columnDefinitions}
             items={shortData}
             selectedRowIds={['id0000012', 'id0000013']}
@@ -99,6 +106,9 @@ export const SingleSelect = () => {
     return (
         <Table
             tableTitle={'Single Select Table'}
+            tableDescription={
+                'Specifies getRowId to avoid the infinite re-rendering loop when onSelectionChange is used'
+            }
             columnDefinitions={columnDefinitions}
             items={shortData}
             multiSelect={false}
@@ -121,6 +131,47 @@ export const GroupBy = () => (
     />
 );
 
+export const GroupBySingleSelect = () => {
+    const getRowId = useCallback((data) => data.id, []);
+    return (
+        <Table
+            tableTitle={'GroupBy Table'}
+            tableDescription={
+                'Specifies getRowId to avoid the infinite re-rendering loop when onSelectionChange is used and use onSelectedRowIdsChange to catch the update'
+            }
+            columnDefinitions={columnDefinitions}
+            items={groupByData}
+            disableGroupBy={false}
+            disableRowSelect={false}
+            multiSelect={false}
+            defaultGroups={['name']}
+            getRowId={getRowId}
+            onSelectedRowIdsChange={action('onSelectedRowIdsChange')}
+        />
+    );
+};
+
+export const GroupByMultiSelect = () => {
+    const getRowId = useCallback((data) => data.id, []);
+    return (
+        <Table
+            tableTitle={'GroupBy Table'}
+            tableDescription={
+                'Specifies getRowId to avoid the infinite re-rendering loop when onSelectionChange is used and use onSelectedRowIdsChange to catch the update'
+            }
+            columnDefinitions={columnDefinitions}
+            selectedRowIds={['id0000012', 'id0000013']}
+            items={groupByData}
+            disableGroupBy={false}
+            disableRowSelect={false}
+            multiSelect={true}
+            defaultGroups={['name']}
+            getRowId={getRowId}
+            onSelectedRowIdsChange={action('onSelectedRowIdsChange')}
+        />
+    );
+};
+
 export const ColumnFilters = () => (
     <Table
         tableTitle={'Column Filter Table'}
@@ -139,7 +190,7 @@ export const ExpandedTable = () => {
             tableTitle={'Expanded Table'}
             columnDefinitions={columnDefinitions}
             getRowId={getRowId}
-            items={groupByData}
+            items={expandableData}
             disableExpand={false}
             onSelectionChange={action('onSelectionChange')}
             onSelectedRowIdsChange={action('onSelectedRowIdsChange')}
@@ -202,7 +253,7 @@ export const RemoteFetch = () => {
     const fetchIdRef = React.useRef(0);
     const getRowId = useCallback((data) => data.id, []);
     const data = useMemo<DataType[]>(() => {
-        const data = [];
+        const data: DataType[] = [];
         for (let i = 0; i < 1000; i++) {
             data.push({
                 id: i.toString(),