Issue 591 - Row and column selection with multiple tables (#594)

Marc-Andre-Rivet · Shammamah Hossain · commit 187f652562b6 · 2019-09-20T11:18:33.000-04:00
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -8,6 +8,9 @@ This project adheres to [Semantic Versioning](http://semver.org/).
 - The `datestartswith` relational operator now supports number comparison
 - Fixed a bug where the implicit operator for columns was `equal` instead of the expected default for the column type
 
+[#591](https://github.com/plotly/dash-table/issues/591)
+- Fixed row and column selection when multiple tables are present
+
 ## [4.3.0] - 2019-09-17
 ### Added
 [#566](https://github.com/plotly/dash-table/pull/566)
diff --git a/demo/App.tsx b/demo/App.tsx
@@ -6,7 +6,6 @@ import Environment from 'core/environment';
 import { memoizeOne } from 'core/memoizer';
 import Logger from 'core/Logger';
 import AppState, { AppMode, AppFlavor } from './AppMode';
-import memoizerCache from 'core/cache/memoizer';
 
 import './style.less';
 
@@ -50,14 +49,18 @@ class App extends Component<any, any> {
                     }} />
             </div>);
         } else if (mode === AppMode.TaleOfTwoTables) {
-            const props: any = {};
-            Object.entries(this.state.tableProps).forEach(([key, value]) => {
-                props[key] = this.propCache.get(key)(value);
-            });
+            if (!this.state.tableProps2) {
+                this.setState({
+                    tableProps2: R.clone(this.state.tableProps)
+                });
+            }
 
-            return (<DataTable
-                {...props}
-            />);
+            const baseId = this.state.tableProps2 && this.state.tableProps2.id;
+
+            return (this.state.tableProps2 ? <DataTable
+                {...this.state.tableProps2}
+                id={baseId ? 'table2' : baseId}
+            /> : null);
         }
     }
 
@@ -71,8 +74,6 @@ class App extends Component<any, any> {
         </div>);
     }
 
-    private propCache = memoizerCache<[string]>()(R.clone);
-
     private setProps = memoizeOne(() => {
         return (newProps: any) => {
             Logger.debug('--->', newProps);
diff --git a/demo/AppMode.ts b/demo/AppMode.ts
@@ -36,7 +36,8 @@ export enum AppFlavor {
     FixedColumnPlus1 = 'fixed_columns={ "headers": true, "data": 1 }',
     FixedRow = 'fixed_rows={ "headers": true }',
     FixedRowPlus1 = 'fixed_rows={ "headers": true, "data": 1 }',
-    Merged = 'merge_duplicate_headers=true'
+    Merged = 'merge_duplicate_headers=true',
+    NoId = 'id=null'
 }
 
 export const ReadWriteModes = [
@@ -346,6 +347,7 @@ function getModeState(mode: string | null) {
         case AppMode.SingleHeaders:
             return getSingleHeaderState();
         case AppMode.TaleOfTwoTables:
+            return getActionableState();
         case AppMode.Default:
         default:
             return getDefaultState();
diff --git a/src/dash-table/components/CellFactory.tsx b/src/dash-table/components/CellFactory.tsx
@@ -42,6 +42,7 @@ export default class CellFactory {
             dropdown,
             data,
             dropdown_data,
+            id,
             is_focused,
             row_deletable,
             row_selectable,
@@ -94,6 +95,7 @@ export default class CellFactory {
         );
 
         const operations = this.cellOperations(
+            id,
             data,
             virtualized.data,
             virtualized.indices,
diff --git a/src/dash-table/components/HeaderFactory.tsx b/src/dash-table/components/HeaderFactory.tsx
@@ -40,6 +40,7 @@ export default class HeaderFactory {
             columns,
             data,
             hidden_columns,
+            id,
             map,
             merge_duplicate_headers,
             page_action,
@@ -93,6 +94,7 @@ export default class HeaderFactory {
         );
 
         const contents = this.headerContent(
+            id,
             visibleColumns,
             columns,
             hidden_columns,
diff --git a/src/dash-table/derived/cell/operations.tsx b/src/dash-table/derived/cell/operations.tsx
@@ -33,7 +33,7 @@ function deleteRow(idx: number, data: Data, selectedRows: number[]) {
     return newProps;
 }
 
-function rowSelectCell(idx: number, rowSelectable: Selection, selectedRows: number[], setProps: SetProps, data: Data) {
+function rowSelectCell(id: string, idx: number, rowSelectable: Selection, selectedRows: number[], setProps: SetProps, data: Data) {
     return (<td
         key='select'
         className='dash-select-cell'
@@ -42,7 +42,7 @@ function rowSelectCell(idx: number, rowSelectable: Selection, selectedRows: numb
         <input
             type={rowSelectable === 'single' ? 'radio' : 'checkbox'}
             style={{ verticalAlign: 'middle' }}
-            name='row-select'
+            name={`row-select-${id}`}
             checked={R.includes(idx, selectedRows)}
             onChange={() => {
                 const newSelectedRows = rowSelectable === 'single' ?
@@ -73,6 +73,7 @@ function rowDeleteCell(doDelete: () => any) {
 }
 
 const getter = (
+    id: string,
     data: Data,
     viewportData: Data,
     viewportIndices: Indices,
@@ -83,7 +84,7 @@ const getter = (
 ): JSX.Element[][] => R.addIndex<Datum, JSX.Element[]>(R.map)(
     (_, rowIndex) => [
         ...(rowDeletable ? [rowDeleteCell(() => setProps(deleteRow(viewportIndices[rowIndex], data, selectedRows)))] : []),
-        ...(rowSelectable ? [rowSelectCell(viewportIndices[rowIndex], rowSelectable, selectedRows, setProps, data)] : [])
+        ...(rowSelectable ? [rowSelectCell(id, viewportIndices[rowIndex], rowSelectable, selectedRows, setProps, data)] : [])
     ],
     viewportData
 );
diff --git a/src/dash-table/derived/header/content.tsx b/src/dash-table/derived/header/content.tsx
@@ -150,6 +150,7 @@ function getSortingIcon(columnId: ColumnId, sortBy: SortBy) {
 }
 
 function getter(
+    id: string,
     visibleColumns: Columns,
     columns: Columns,
     hiddenColumns: string[] | undefined,
@@ -230,7 +231,7 @@ function getter(
                                         column_selectable === 'single',
                                         !allSelected
                                     )}
-                                    name='column-header--select'
+                                    name={`column-select-${id}`}
                                     type={column_selectable === 'single' ?
                                         'radio' :
                                         'checkbox'
diff --git a/tests/cypress/src/DashTable.ts b/tests/cypress/src/DashTable.ts
@@ -19,104 +19,110 @@ const getSelector = (state: State) => {
     }
 };
 
-export default class DashTable {
-    static getCell(row: number, column: number, editable: State = State.Ready) {
-        return cy.get(`#table ${getSelector(editable)} tbody tr td.column-${column}`).eq(row);
+export class DashTableHelper {
+    constructor(private readonly id) {
+
+    }
+
+    public getCell(row: number, column: number, editable: State = State.Ready) {
+        return cy.get(`#${this.id} ${getSelector(editable)} tbody tr td.column-${column}`).eq(row);
     }
 
-    static getCellById(row: number, column: string, editable: State = State.Ready) {
-        return cy.get(`#table ${getSelector(editable)} tbody tr td[data-dash-column="${column}"]`).eq(row);
+    public getCellById(row: number, column: string, editable: State = State.Ready) {
+        return cy.get(`#${this.id} ${getSelector(editable)} tbody tr td[data-dash-column="${column}"]`).eq(row);
     }
 
-    static getFilter(column: number, editable: State = State.Ready) {
-        return cy.get(`#table ${getSelector(editable)} tbody tr th.dash-filter.column-${column}`);
+    public getFilter(column: number, editable: State = State.Ready) {
+        return cy.get(`#${this.id} ${getSelector(editable)} tbody tr th.dash-filter.column-${column}`);
     }
 
-    static getFilterById(column: string, editable: State = State.Ready) {
-        return cy.get(`#table ${getSelector(editable)} tbody tr th.dash-filter[data-dash-column="${column}"]`);
+    public getFilterById(column: string, editable: State = State.Ready) {
+        return cy.get(`#${this.id} ${getSelector(editable)} tbody tr th.dash-filter[data-dash-column="${column}"]`);
     }
 
-    static getHeader(row: number, column: number, editable: State = State.Ready) {
-        return cy.get(`#table ${getSelector(editable)} tbody tr th.dash-header.column-${column}`).eq(row);
+    public getHeader(row: number, column: number, editable: State = State.Ready) {
+        return cy.get(`#${this.id} ${getSelector(editable)} tbody tr th.dash-header.column-${column}`).eq(row);
     }
 
-    static getHeaderById(row: number, column: string, editable: State = State.Ready) {
-        return cy.get(`#table ${getSelector(editable)} tbody tr th.dash-header[data-dash-column="${column}"]`).eq(row);
+    public getHeaderById(row: number, column: string, editable: State = State.Ready) {
+        return cy.get(`#${this.id} ${getSelector(editable)} tbody tr th.dash-header[data-dash-column="${column}"]`).eq(row);
     }
 
-    static focusCell(row: number, column: number) {
+    public focusCell(row: number, column: number) {
         // somehow we need to scrollIntoView AFTER click, or it doesn't
         // work right. Why?
         return this.getCell(row, column).click().scrollIntoView();
     }
 
-    static focusCellById(row: number, column: string) {
+    public focusCellById(row: number, column: string) {
         return this.getCellById(row, column).click().scrollIntoView();
     }
 
-    static clearColumnById(row: number, column: string, editable: State = State.Ready) {
-        return cy.get(`#table ${getSelector(editable)} tbody tr th.dash-header[data-dash-column="${column}"] .column-header--clear`).eq(row).click();
+    public clearColumnById(row: number, column: string, editable: State = State.Ready) {
+        return cy.get(`#${this.id} ${getSelector(editable)} tbody tr th.dash-header[data-dash-column="${column}"] .column-header--clear`).eq(row).click();
     }
 
-    static deleteColumnById(row: number, column: string, editable: State = State.Ready) {
-        return cy.get(`#table ${getSelector(editable)} tbody tr th.dash-header[data-dash-column="${column}"] .column-header--delete`).eq(row).click();
+    public deleteColumnById(row: number, column: string, editable: State = State.Ready) {
+        return cy.get(`#${this.id} ${getSelector(editable)} tbody tr th.dash-header[data-dash-column="${column}"] .column-header--delete`).eq(row).click();
     }
 
-    static hideColumnById(row: number, column: string, editable: State = State.Ready) {
-        return cy.get(`#table ${getSelector(editable)} tbody tr th.dash-header[data-dash-column="${column}"] .column-header--hide`).eq(row).click();
+    public hideColumnById(row: number, column: string, editable: State = State.Ready) {
+        return cy.get(`#${this.id} ${getSelector(editable)} tbody tr th.dash-header[data-dash-column="${column}"] .column-header--hide`).eq(row).click();
     }
 
-    static getSelectColumnById(row: number, column: string, editable: State = State.Ready) {
-        return cy.get(`#table ${getSelector(editable)} tbody tr th.dash-header[data-dash-column="${column}"] .column-header--select input`).eq(row);
+    public getSelectColumnById(row: number, column: string, editable: State = State.Ready) {
+        return cy.get(`#${this.id} ${getSelector(editable)} tbody tr th.dash-header[data-dash-column="${column}"] .column-header--select input`).eq(row);
     }
 
-    static selectColumnById(row: number, column: string) {
-        return DashTable.getSelectColumnById(row, column).click();
+    public selectColumnById(row: number, column: string) {
+        return this.getSelectColumnById(row, column).click();
     }
 
-    static getDelete(row: number, editable: State = State.Ready) {
-        return cy.get(`#table ${getSelector(editable)} tbody tr td.dash-delete-cell`).eq(row);
+    public getDelete(row: number, editable: State = State.Ready) {
+        return cy.get(`#${this.id} ${getSelector(editable)} tbody tr td.dash-delete-cell`).eq(row);
     }
 
-    static getSelect(row: number, editable: State = State.Ready) {
-        return cy.get(`#table ${getSelector(editable)} tbody tr td.dash-select-cell`).eq(row);
+    public getSelect(row: number, editable: State = State.Ready) {
+        return cy.get(`#${this.id} ${getSelector(editable)} tbody tr td.dash-select-cell`).eq(row);
     }
 
-    static getActiveCell(editable: State = State.Ready) {
-        return cy.get(`#table ${getSelector(editable)} tbody td.focused`);
+    public getActiveCell(editable: State = State.Ready) {
+        return cy.get(`#${this.id} ${getSelector(editable)} tbody td.focused`);
     }
 
-    static getSelectedCells(editable: State = State.Ready) {
-        return cy.get(`#table ${getSelector(editable)} tbody td.cell--selected`);
+    public getSelectedCells(editable: State = State.Ready) {
+        return cy.get(`#${this.id} ${getSelector(editable)} tbody td.cell--selected`);
     }
 
-    static scrollToTop() {
+    public scrollToTop() {
         cy.get(`.cell.cell-1-1.dash-fixed-content`).invoke(`outerHeight`).then(height => {
             cy.scrollTo(0, -height);
         });
     }
 
-    static scrollToBottom() {
+    public scrollToBottom() {
         cy.get(`.cell.cell-1-1.dash-fixed-content`).invoke(`outerHeight`).then(height => {
             cy.scrollTo(0, height);
         });
     }
 
-    static getCellInLastRowOfColumn(column: number) {
+    public getCellInLastRowOfColumn(column: number) {
         const cellInLastRow = cy.get(`td.dash-cell.column-${column}`).last().then(elem => {
             const lastRow = elem ? elem.attr(`data-dash-row`) : undefined;
             return lastRow ? cy.get(`td.dash-cell.column-${column}[data-dash-row="${lastRow}"`) : undefined;
         });
         return cellInLastRow;
     }
 
-    static getCellFromDataDash(row: number, column: number) {
+    public getCellFromDataDash(row: number, column: number) {
         return cy.get(`td.column-${column}[data-dash-row="${row}"]`);
     }
 
-    static toggleScroll(toggled: boolean) {
+    public toggleScroll(toggled: boolean) {
         cy.get('.row-1').then($el => {
             $el[0].style.overflow = toggled ? '' : 'unset';
         });
     }
 }
+
+export default new DashTableHelper('table');
diff --git a/tests/cypress/tests/standalone/select_row_column_test.ts b/tests/cypress/tests/standalone/select_row_column_test.ts
@@ -0,0 +1,79 @@
+import DashTable, { DashTableHelper } from 'cypress/DashTable';
+
+import { BasicModes, AppMode, AppFlavor } from 'demo/AppMode';
+
+Object.values(BasicModes).forEach(mode => {
+    describe(`select row, mode=${mode}`, () => {
+        beforeEach(() => {
+            cy.visit(`http://localhost:8080?mode=${mode}`);
+            DashTable.toggleScroll(false);
+        });
+
+        describe('fe pagination & sort', () => {
+            it('can select row', () => {
+                DashTable.getSelect(0).within(() => cy.get('input').click());
+                DashTable.getSelect(0).within(() => cy.get('input').should('be.checked'));
+            });
+
+            it('can select row when sorted', () => {
+                cy.get('tr th.column-0 .column-header--sort').last().click({ force: true });
+                DashTable.getSelect(0).within(() => cy.get('input').click());
+                DashTable.getSelect(0).within(() => cy.get('input').should('be.checked'));
+            });
+
+            it('select, sort, new row is not selected', () => {
+                DashTable.getSelect(0).within(() => cy.get('input').click());
+                DashTable.getSelect(0).within(() => cy.get('input').should('be.checked'));
+                cy.get('tr th.column-0 .column-header--sort').last().click({ force: true }).click({ force: true });
+                DashTable.getSelect(0).within(() => cy.get('input').should('not.be.checked'));
+            });
+        });
+    });
+});
+
+describe('select rows & columns in multiple tables without id', () => {
+    let table1: DashTableHelper;
+    let table2: DashTableHelper;
+
+    beforeEach(() => {
+        cy.visit(`http://localhost:8080?mode=${AppMode.TaleOfTwoTables}&flavor=${[AppFlavor.ColumnSelectableSingle,AppFlavor.NoId].join(';')}`).then(() => {
+            const ids: string[] = [];
+            return cy.get('.dash-spreadsheet-container').parent().each(el => {
+                const id = el.attr('id');
+                if (id) {
+                    ids.push(id);
+                }
+            }).then(() => ids);
+        }).then(ids => {
+            cy.log('table ids', ids);
+            expect(ids.length).to.equal(2);
+
+            table1 = new DashTableHelper(ids[0]);
+            table2 = new DashTableHelper(ids[1]);
+        });
+
+        DashTable.toggleScroll(false);
+    });
+
+    it('can select a row in both tables', () => {
+        table1.getSelect(0).within(() => cy.get('input').click());
+        table1.getSelect(0).within(() => cy.get('input').should('be.checked'));
+
+        table2.getSelect(1).within(() => cy.get('input').click());
+        table2.getSelect(1).within(() => cy.get('input').should('be.checked'));
+
+        table1.getSelect(0).within(() => cy.get('input').should('be.checked'));
+        table1.getSelect(1).within(() => cy.get('input').should('not.be.checked'));
+    });
+
+    it('can select a column in both tables', () => {
+        table1.selectColumnById(0, 'ccc');
+        table1.getSelectColumnById(2, 'ccc').should('be.checked');
+
+        table2.selectColumnById(0, 'rows');
+        table2.getSelectColumnById(2, 'rows').should('be.checked');
+
+        table1.getSelectColumnById(2, 'ccc').should('be.checked');
+        table1.getSelectColumnById(2, 'rows').should('not.be.checked');
+    });
+});
diff --git a/tests/cypress/tests/standalone/select_row_test.ts b/tests/cypress/tests/standalone/select_row_test.ts
