Fix table selection (#695)

nexxtway · Apr 24, 2019 · babf4c8 · babf4c8
1 parent 9b9ceae
commit babf4c8
Show file tree

Hide file tree

Showing 16 changed files with 249 additions and 67 deletions.
diff --git a/integration/specs/Table/table-5.spec.js b/integration/specs/Table/table-5.spec.js
@@ -14,6 +14,7 @@ describe('Table with selection', () => {
     });
     it('should select and then deselect a row', () => {
         const table = new PageTable(TABLE);
+        table.waitUntilDataIsLoaded();
         const row5 = table.getRow(4);
         expect(row5.isRowSelected()).toBe(false);
         row5.selectRow();
@@ -23,6 +24,7 @@ describe('Table with selection', () => {
     });
     it('should select all rows', () => {
         const table = new PageTable(TABLE);
+        table.waitUntilDataIsLoaded();
         const row1 = table.getRow(0);
         const row2 = table.getRow(1);
         const row3 = table.getRow(2);
@@ -48,6 +50,7 @@ describe('Table with selection', () => {
     });
     it('should deselect all rows', () => {
         const table = new PageTable(TABLE);
+        table.waitUntilDataIsLoaded();
         const row1 = table.getRow(0);
         const row2 = table.getRow(1);
         const row3 = table.getRow(2);
@@ -67,6 +70,7 @@ describe('Table with selection', () => {
     });
     it('should select the second and fifth row', () => {
         const table = new PageTable(TABLE);
+        table.waitUntilDataIsLoaded();
         const row1 = table.getRow(0);
         const row2 = table.getRow(1);
         const row3 = table.getRow(2);
@@ -86,6 +90,7 @@ describe('Table with selection', () => {
     });
     it('should deselect all rows when the second and fifth row are selected and then deselect all rows', () => {
         const table = new PageTable(TABLE);
+        table.waitUntilDataIsLoaded();
         const row1 = table.getRow(0);
         const row2 = table.getRow(1);
         const row3 = table.getRow(2);
@@ -106,6 +111,7 @@ describe('Table with selection', () => {
     });
     it('should select all rows between selected rows while press shift key', () => {
         const table = new PageTable(TABLE);
+        table.waitUntilDataIsLoaded();
         const row1 = table.getRow(0);
         const row2 = table.getRow(1);
         const row3 = table.getRow(2);
@@ -127,6 +133,7 @@ describe('Table with selection', () => {
     });
     it('should deselect all rows between the two rows deselected while press shift key', () => {
         const table = new PageTable(TABLE);
+        table.waitUntilDataIsLoaded();
         const row1 = table.getRow(0);
         const row2 = table.getRow(1);
         const row3 = table.getRow(2);

diff --git a/src/components/Modal/pageObject/index.js b/src/components/Modal/pageObject/index.js
@@ -50,15 +50,15 @@ class PageModal {
     }
 
     /**
-     * Waiting until the open modal transition has finished.
+     * Wait until the open modal transition has finished.
      * @method
      */
     waitUntilOpen() {
         browser.waitUntil(() => this.isOpen());
     }
 
     /**
-     * Waiting until the close modal transition has finished.
+     * Wait until the close modal transition has finished.
      * @method
      */
     waitUntilClose() {

diff --git a/src/components/Table/__test__/table.spec.js b/src/components/Table/__test__/table.spec.js
@@ -853,4 +853,66 @@ describe('<Table />', () => {
         }]);
         expect(component.instance().lastSelectedRowKey).toBe('1234qwerty');
     });
+    it('should set the right indexes when data prop changes', () => {
+        const component = mount(
+            <Table
+                data={[]}
+                showCheckboxColumn
+                keyField="id">
+
+                <Column field="name" header="Name" />
+            </Table>,
+        );
+        component.setProps({
+            data: tableData,
+        });
+        expect(component.instance().indexes).toEqual({
+            '1234qwerty': { rowIndex: 0 },
+            '5678asdfgh': { rowIndex: 1 },
+            '9012zxcvbn': { rowIndex: 2 },
+        });
+    });
+    it('should set the right state when data prop changes', () => {
+        const component = mount(
+            <Table
+                data={[]}
+                showCheckboxColumn
+                keyField="id">
+
+                <Column field="name" header="Name" />
+            </Table>,
+        );
+        component.setProps({
+            data: tableData,
+        });
+        const { state } = component.instance();
+        expect(state.rows).toEqual([
+            { inputType: 'checkbox', isDisabled: false, isSelected: false, key: '1234qwerty' },
+            { inputType: 'checkbox', isDisabled: false, isSelected: false, key: '5678asdfgh' },
+            { inputType: 'checkbox', isDisabled: false, isSelected: false, key: '9012zxcvbn' },
+        ]);
+        expect(state.bulkSelection).toBe('none');
+    });
+    it('should set the right state when data prop changes and have selected rows', () => {
+        const component = mount(
+            <Table
+                data={[]}
+                showCheckboxColumn
+                selectedRows={['5678asdfgh']}
+                keyField="id">
+
+                <Column field="name" header="Name" />
+            </Table>,
+        );
+        component.setProps({
+            data: tableData,
+        });
+        const { state } = component.instance();
+        expect(state.rows).toEqual([
+            { inputType: 'checkbox', isDisabled: false, isSelected: false, key: '1234qwerty' },
+            { inputType: 'checkbox', isDisabled: false, isSelected: true, key: '5678asdfgh' },
+            { inputType: 'checkbox', isDisabled: false, isSelected: false, key: '9012zxcvbn' },
+        ]);
+        expect(state.bulkSelection).toBe('some');
+    });
 });
diff --git a/src/components/Table/body/__test__/body.spec.js b/src/components/Table/body/__test__/body.spec.js
@@ -33,14 +33,22 @@ const rows = [
 ];
 
 describe('<Body />', () => {
-    it('should return an empty component if data and columns are not passed and is not loading', () => {
+    it('should return an empty component when data and columns are not passed and is not loading', () => {
         const component = mount(<Body isLoading={false} />);
         expect(component.find('Empty').exists()).toBe(true);
     });
-    it('should return an empty component if data or columns are not arrays and is not loading', () => {
-        const component = mount(<Body columns={{}} data={[]} isLoading={false} />);
+    it('should return an empty component when there is not data and is not loading', () => {
+        const component = mount(<Body columns={[{}]} data={[]} isLoading={false} />);
         expect(component.find('Empty').exists()).toBe(true);
     });
+    it('should return a loading component when there is not data and is loading', () => {
+        const component = mount(<Body columns={[{}]} data={[]} isLoading />);
+        expect(component.find('Loading').exists()).toBe(true);
+    });
+    it('should return one row more when there is data and is loading', () => {
+        const component = mount(<Body columns={columns} data={data} isLoading />);
+        expect(component.children().length).toBe(3);
+    });
     it('should return an array of Row components', () => {
         const component = mount(<Body data={data} columns={columns} rows={rows} />);
         const rowElements = component.find('Row');

diff --git a/src/components/Table/body/__test__/loadingCells.spec.js b/src/components/Table/body/__test__/loadingCells.spec.js
@@ -0,0 +1,18 @@
+import React from 'react';
+import { mount } from 'enzyme';
+import LoadingCells from '../loadingCells';
+
+describe('<LoadingCells />', () => {
+    it('should not render anything when value is 0', () => {
+        const component = mount(
+            <LoadingCells value={0} />,
+        );
+        expect(component.children().length).toBe(0);
+    });
+    it('should render the amount of children that match with the value passed', () => {
+        const component = mount(
+            <LoadingCells value={3} />,
+        );
+        expect(component.children().length).toBe(3);
+    });
+});
diff --git a/src/components/Table/body/__test__/row.spec.js b/src/components/Table/body/__test__/row.spec.js
@@ -92,4 +92,11 @@ describe('<Row />', () => {
         expect(cell.at(1).prop('isFirst')).toBe(true);
         expect(cell.at(2).prop('isFirst')).toBe(false);
     });
+    it('should render LoadingCells component when data type is LOADING', () => {
+        const rowData = { type: 'LOADING' };
+        const component = mount(
+            <Row data={rowData} columns={columns} />,
+        );
+        expect(component.find('LoadingCells').exists()).toBe(true);
+    });
 });
diff --git a/src/components/Table/body/index.js b/src/components/Table/body/index.js
@@ -32,7 +32,6 @@ export default class Body extends PureComponent {
             emptyTitle,
             emptyDescription,
         } = this.props;
-        const hasData = Array.isArray(data) && data.length > 0;
         const isEmpty = data.length === 0;
         const columnsLength = columns.length;
 
@@ -50,34 +49,31 @@ export default class Body extends PureComponent {
             );
         }
 
-        if (hasData && Array.isArray(columns)) {
-            const newData = getData(data, isLoading);
-            return newData.map((item, index) => {
-                const row = rows[index];
-                const rowKeyValue = rows[index] && rows[index].key;
-                const key = rowKeyValue || `row-${index + 1}`;
+        const newData = getData(data, isLoading);
+        return newData.map((item, index) => {
+            const row = rows[index];
+            const rowKeyValue = rows[index] && rows[index].key;
+            const key = rowKeyValue || `row-${index + 1}`;
 
-                return (
-                    <Row
-                        {...row}
-                        key={key}
-                        data={item}
-                        columns={columns}
-                        tableId={tableId}
-                        onSelectRow={(event, isMultiple) => onSelectRow(
+            return (
+                <Row
+                    {...row}
+                    key={key}
+                    data={item}
+                    columns={columns}
+                    tableId={tableId}
+                    onSelectRow={(event, isMultiple) => onSelectRow(
                             event,
                             isMultiple,
                             rowKeyValue,
                         )}
-                        onDeselectRow={(event, isMultiple) => onDeselectRow(
+                    onDeselectRow={(event, isMultiple) => onDeselectRow(
                             event,
                             isMultiple,
                             rowKeyValue,
                         )} />
-                );
-            });
-        }
-        return null;
+            );
+        });
     }
 }
 

diff --git a/src/components/Table/body/loadingCells.js b/src/components/Table/body/loadingCells.js
@@ -1,4 +1,5 @@
 import React from 'react';
+import PropTypes from 'prop-types';
 
 const widths = ['40%', '60%', '70%', '85%', '95%'];
 
@@ -7,18 +8,30 @@ function getRandomWidth() {
 }
 
 export default function LoadingCells({ value }) {
-    return Array(value).fill().map((item, index) => {
-        const key = `loading-cell-${index}`;
-        const styles = {
-            width: getRandomWidth(),
-        };
+    if (value > 0) {
+        return Array(value).fill().map((item, index) => {
+            const key = `loading-cell-${index}`;
+            const styles = {
+                width: getRandomWidth(),
+            };
 
-        return (
-            <td key={key}>
-                <div className="rainbow-table_body--loading">
-                    <div className="rainbow-table_body-element--loading" style={styles} />
-                </div>
-            </td>
-        );
-    });
+            return (
+                <td key={key}>
+                    <div className="rainbow-table_body--loading">
+                        <div className="rainbow-table_body-element--loading" style={styles} />
+                    </div>
+                </td>
+            );
+        });
+    }
+    return null;
 }
+
+LoadingCells.propTypes = {
+    value: PropTypes.number,
+};
+
+LoadingCells.defaultProps = {
+    value: 0,
+};
+
diff --git a/src/components/Table/helpers/columns/__test__/getColumns.spec.js b/src/components/Table/helpers/columns/__test__/getColumns.spec.js
@@ -8,6 +8,11 @@ describe('getColumns', () => {
         const showCheckboxColumn = false;
         expect(getColumns(columns, showCheckboxColumn)).toBeNull();
     });
+    it('should return an empty array when columns is not passed', () => {
+        const columns = undefined;
+        const showCheckboxColumn = false;
+        expect(getColumns(columns, showCheckboxColumn)).toEqual([]);
+    });
     it('should return an empty array when columns is an array with falsy values and showCheckboxColumn is false', () => {
         const columns = [null, undefined];
         const showCheckboxColumn = false;

diff --git a/src/components/Table/helpers/columns/getColumns.js b/src/components/Table/helpers/columns/getColumns.js
@@ -1,7 +1,7 @@
 import React from 'react';
 import { SELECTABLE_CHECKBOX } from './';
 
-export default function getColumns(columns, showCheckboxColumn) {
+export default function getColumns(columns = [], showCheckboxColumn) {
     const columnsData = React.Children.map(columns, (column) => {
         if (column) {
             return column.props;

diff --git a/src/components/Table/helpers/data/__test__/normalizeData.spec.js b/src/components/Table/helpers/data/__test__/normalizeData.spec.js
@@ -0,0 +1,14 @@
+import { normalizeData } from '../';
+
+describe('normalizeData', () => {
+    it('should return the same data passed when it is an empty array', () => {
+        expect(normalizeData([])).toEqual([]);
+    });
+    it('should return the same data passed when it is an array', () => {
+        expect(normalizeData(['data-1', 'data-2'])).toEqual(['data-1', 'data-2']);
+    });
+    it('should return an empty array when data passed is not an array', () => {
+        const values = [null, undefined, {}, 'asd', 123, () => {}];
+        values.forEach(value => expect(normalizeData(value)).toEqual([]));
+    });
+});
diff --git a/src/components/Table/helpers/data/index.js b/src/components/Table/helpers/data/index.js
@@ -0,0 +1,8 @@
+/* eslint-disable import/prefer-default-export */
+
+export function normalizeData(data) {
+    if (Array.isArray(data)) {
+        return data;
+    }
+    return [];
+}