Skip to content
This repository was archived by the owner on Jun 4, 2024. It is now read-only.

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

Merged
merged 5 commits into from
Sep 19, 2019

Conversation

Marc-Andre-Rivet
Copy link
Contributor

@Marc-Andre-Rivet Marc-Andre-Rivet commented Sep 19, 2019

Fixes #591

Marc-André Rivet added 2 commits September 19, 2019 15:19
- additional tests against tables without id
@chriddyp chriddyp temporarily deployed to dash-table-review-pr-594 September 19, 2019 19:51 Inactive
return (this.state.tableProps2 ? <DataTable
{...this.state.tableProps2}
id={baseId ? 'table2' : baseId}
/> : null);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Change the 2 tables test behavior so that

  • the props are not shared between the tables 🙄
  • the 2nd table will have an id only if the 1st one has one

@@ -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}`}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Use the table's id to make the group unique

@@ -230,7 +231,7 @@ function getter(
column_selectable === 'single',
!allSelected
)}
name='column-header--select'
name={`column-select-${id}`}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Use the table's id to make the group unique

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 {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Generalize the helper to work for multiple tables - Leaving the static implementation below for existing tests

});
});

describe('select rows & columns in multiple tables without id', () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Renamed the file to take into consideration that there are now rows and columns tests.

Create two tables without ids and check that row and column selections are not shared between them.

@Marc-Andre-Rivet Marc-Andre-Rivet marked this pull request as ready for review September 19, 2019 19:54
@chriddyp chriddyp temporarily deployed to dash-table-review-pr-594 September 19, 2019 20:02 Inactive
@chriddyp chriddyp temporarily deployed to dash-table-review-pr-594 September 19, 2019 20:07 Inactive
cy.get('.row-1').then($el => {
$el[0].style.overflow = toggled ? '' : 'unset';
});
}
}

export default class DashTable {
private static __helper = new DashTableHelper('table');
Copy link
Collaborator

Choose a reason for hiding this comment

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

would it not work to just say export default const DashTable = new DashTableHelper('table');?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

😞 very much so

Copy link
Collaborator

@alexcjohnson alexcjohnson left a comment

Choose a reason for hiding this comment

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

Looks great. Just one question about 🌴 but since it's just in the 🐅 it's non-⛔️
💃

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

row/column selectable single applies to all tables on the page
3 participants