Skip to content
This repository was archived by the owner on Aug 29, 2025. It is now read-only.

fix regression + new test#63

Merged
Marc-Andre-Rivet merged 1 commit intomasterfrom
3.0-double-click-regression
Sep 10, 2018
Merged

fix regression + new test#63
Marc-Andre-Rivet merged 1 commit intomasterfrom
3.0-double-click-regression

Conversation

@Marc-Andre-Rivet
Copy link
Copy Markdown
Contributor

The refactoring for fixed rows+columns introduced a regression on double click handling.
Modifying the code to do the same transform that's done on click.

Adding one test for this case.

});

it('can get cell with double click', () => {
DashTable.getCell(3, 3).within(() => cy.get('div').dblclick());
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Not sure why I can click on the cell itself but need to dblclick on the child both handlers are defined on the cell itself... otherwise this does not work.

{'id': 13, 'name': 'Consumer disputed?'}
],
n_fixed_columns=2,
n_fixed_rows=1,
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Changing the test case to have fixed rows/columns as this is necessary for the bug to activate.

Copy link
Copy Markdown
Contributor

@wbrgss wbrgss left a comment

Choose a reason for hiding this comment

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

The refactoring for fixed rows+columns introduced a regression on double click handling.
Modifying the code to do the same transform that's done on click.

Is this what is meant to happen? Because with this PR single click behaviour sets a cell to focused (i.e. highlighted), and a double click sets the cell to active (i.e. editable). I'm happy with this behaviour; it's what I suggested in #62. But this comment makes it a bit unclear, unless I'm misinterpreting the meaning of "transform".

@cldougl
Copy link
Copy Markdown
Member

cldougl commented Sep 10, 2018

afaik

single click behaviour sets a cell to focused (i.e. highlighted), and a double click sets the cell to active (i.e. editable).

is what we want to aim for

@Marc-Andre-Rivet
Copy link
Copy Markdown
Contributor Author

afaik it's also what we want, this code's impact/logic was never touched.
By transform I just mean applying the offset so we hit the correct cell.

@Marc-Andre-Rivet Marc-Andre-Rivet merged commit 692d313 into master Sep 10, 2018
@Marc-Andre-Rivet Marc-Andre-Rivet deleted the 3.0-double-click-regression branch September 12, 2018 11:31
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.

3 participants