fix keyboard navigation after copy/paste#90
Conversation
| /*#if TEST_COPY_PASTE*/ | ||
| this.onCopy(e as any); | ||
| e.preventDefault(); | ||
| /*#endif*/ |
There was a problem hiding this comment.
Default handling of keys ends up changing the focus state, which affects how future keys are handled. 'Enter' is meant to set the focus "deeper" than the table, on the input itself, but not Ctrl+C / Ctrl+V. Exiting explicitly.
There was a problem hiding this comment.
Can you explain what these /*#endif*/ comments are doing? templating in code for different builds?
There was a problem hiding this comment.
Yes. The test build has a special build-time variable defined
dash-table/webpack.test.config.js
Line 3 in 5fd75e3
The loader reads through the files and keeps/removes the blocks based on whether the variable is defined or not. This is useful for defining build specific behavior or configurations that need to be more flexible than overriding an alias/file and/or not have to carry the conditional logic for tests/dev/etc. into production code. Created this loader a while ago when I encountered test cases I couldn't run on Selenium because it wasn't possible to open a file selector and choose a file..
https://www.npmjs.com/package/webpack-preprocessor
There was a problem hiding this comment.
Specifically, in this case, Cypress does not have good support of copy & paste events, so instead of letting the browser handle it, triggering the copy/paste processing manually when the key combinations are hit. This creates a gap between the test impl and the real impl but the alternative is to manual test features.
| DOM.focused.type(Key.ArrowDown); | ||
| DashTable.getCell(4, 3).should('have.class', 'focused'); | ||
| DashTable.getCell(3, 3).should('not.have.class', 'focused'); | ||
| }); |
There was a problem hiding this comment.
This test makes sure that we can navigate out of the cell once Ctrl+C has been pressed
cldougl
left a comment
There was a problem hiding this comment.
💃 works as expected now- thanks for the additional test!
| if (e.keyCode === KEY_CODES.C && ctrlDown && !is_focused) { | ||
| /*#if TEST_COPY_PASTE*/ | ||
| this.onCopy(e as any); | ||
| e.preventDefault(); |
There was a problem hiding this comment.
The more this pops up the more I think we should keep track of custom vs default event handlers.
There was a problem hiding this comment.
Actually this one was already there, hidden away in onCopy, just made both consistent :)
No description provided.