-
-
Notifications
You must be signed in to change notification settings - Fork 73
Issue 790 - Copy/paste regression when selecting cells with mouse #818
Conversation
} | ||
|
||
return false; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am unable to use Ctrl+C/Ctrl+V in a way that triggers copy/paste in Selenium, Chrome does not support document.execCommand('paste')
and the ClipboardAPI requires the user to Allow
browser permission for usage. These reasons are behind the special "test" build and also behind this long standing regression. Prior to this change, the tests worked but the real build didn't. This tries to better align the test behavior vs. the real world behavior.
if (target) { | ||
target.focus(); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Make sure that the active cell is focused if the active element is another data cell in the table (triggered by mouse click to change the selected_cells)
e.preventDefault(); | ||
if (!this.preventCopyPaste()) { | ||
this.onPaste({} as any); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Trigger the test "paste" only if the cell with focus is the active_cell
e.preventDefault(); | ||
if (!this.preventCopyPaste()) { | ||
this.onCopy(e as any); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Trigger the test "copy" only if the cell with focus is the active_cell
target.cell(0, 0).click() | ||
target.cell(2, 0).click() | ||
with test.hold(Keys.SHIFT): | ||
test.send_keys(Keys.ARROW_DOWN + Keys.ARROW_DOWN) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Test navigating to the cell both by selecting a region with the keyboard and the mouse
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wow, that's annoying behavior... but looks like a good fix. 💃
Hey Marc! |
@Chris-Keo this is a merged PR, and this work has been released. Please look through the open issues to see if the behavior you're observing has already been reported - and if not, feel free to submit a new issue and we'll take a look. |
Closes #790