Fix: CellExternalCopyManager plugin restores focus on paste

[PierreYvesR]

When using doing copy-paste with CellExternalCopyManager, the grid loses focus after Ctrl-V.

The issue can be seen on example-excel-compatible-spreadsheet by playing with the arrow keys and doing some copy/paste.

The commit reuses the code found some 20 lines before in the same function to restore the focus.

[ghiscoding]

hmm I would say that I don't want any new console log, I assume you mainly added it for testing so please remove it. Also the 100ms seems like a lot, can't you go with a much smaller number? @6pac what do you think about this PR?

[PierreYvesR]

I've removed the console log.
I had put it in as there's already one at Line 430, FWIW I've actually never seen those logs in my console ;-)

As for the 100ms delay, it was already there before this commit, so I can't really comment. Happy to change it though.

[PierreYvesR]

On a closer look, I'm wondering whether 100ms should be replaced with

this._options?.clipboardPasteDelay ?? CLIPBOARD_PASTE_DELAY

[ghiscoding]

On a closer look, I'm wondering whether 100ms should be replaced with

this._options?.clipboardPasteDelay ?? CLIPBOARD_PASTE_DELAY

yeah sure if you want to add an extra config option for it, that should be ok but please update the associated TypeScript interface for it too

[PierreYvesR]

The option is already in there, I would just use it for both delays (delay on paste and delay on copy).

[ghiscoding]

if it's the same kind of topic, then I guess it's ok

[ghiscoding]

@PierreYvesR before I merged this, can I assume that you tried it after the latest code change?

[PierreYvesR]

Yes I did. I also played with the timeout delay, and it does work. I can even set it to 0 without issues.

I've written a Cypress test for this (I had never used Cypress, so I was intrigued).
However it seems I would need to add cypress-real-events plugin to be able to send Copy and Paste keyboard event.

[ghiscoding]

you can add it if you want, just make sure that it's for devDependencies. I added Cypress because that's a good way to make sure everything is working and new PRs aren't causing regressions. It would not have been possible to remove jQuery/jQueryUI and migrate to TypeScript without such tests in place... well I mean we could have but we would have certainly faced a lot more bugs on new releases, so as you can see, these E2E tests are really important to make sure any new PRs aren't introducing new regressions and we fix bugs early :)

For Copy+Paste, these 2 articles seems interesting, I used the invoke in a few tests

• https://egghead.io/blog/handling-copy-and-paste-in-cypress
• https://dev.to/walmyrlimaesilv/testing-copy-to-clipboard-with-cypress-1414

[PierreYvesR]

https://egghead.io/blog/handling-copy-and-paste-in-cypress

That's where I found cypress-real-events (at the very end of the article)

[ghiscoding]

great thanks for the contribution and new Cypress test :) Does that complete your code change?

Merci :)

[PierreYvesR]

Yes, but I guess I should squash it all into one commit, right?

Merci :)
Thank you for all the work on SlickGrid.

[ghiscoding]

no need GitHub has a "Squash and Merge" that I use most often, thanks again for the contribution. However note that since I already published a new release just couple days ago, I will probably wait a few more days to release this, maybe next weekend. Remind me if I forget, cheers

[6pac]

All looks good to me :-)

[zewa666]

@PierreYvesR thanks for mentioning cypress-real-events. Thats a real gamechanger to test copy&paste events properly

[ghiscoding]

Thanks for the contribution, this was released in v5.9.1.