Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Copy paste not working #242

Closed
dcalde opened this issue May 16, 2018 · 7 comments
Closed

Copy paste not working #242

dcalde opened this issue May 16, 2018 · 7 comments

Comments

@dcalde
Copy link

dcalde commented May 16, 2018

I haven't downloaded the code and tried locally with the latest version yet, but the copy paste examples http://6pac.github.com/SlickGrid/examples/example-spreadsheet.html are not working for me (Chrome 66 & Edge 42).

@6pac
Copy link
Owner

6pac commented May 17, 2018

Thanks for the heads up. This appears to be because of the recent PR 241.

@ghiscoding ? (sorry, I know you're busy with the fixed column branch!)

example-spreadsheet is broken in several ways, but interestingly example-excel-compatible-spreadsheet appears to still work.

If this can't be resolved soon, perhaps I need to revert these changes for a while until this is sorted out. It's what I was worrried about with #68, when I looked at it, there was no clear way to fix that issue without breaking other use cases.

@ghiscoding
Copy link
Collaborator

ghiscoding commented May 17, 2018

The provided example does not work with Copy and Paste even after reverting (by hand) my code. Was that example ever working with Copy Paste? I mean I went and manually added some data in some of the cells (in the grid) and did a Copy and Pasted in Excel and nothing happens.

example-spreadsheet is broken in several ways

in what ways?

but interestingly example-excel-compatible-spreadsheet appears to still work.

I have the latest SlickGrid version in my Angular-Slickgrid and the Copy Paste still works correctly.
I think the example-spreadsheet itself is missing something for the Copy and Paste to work

EDIT
The problem seems to be in the CellCopyManager Plugin, if you replace it by the following, it starts working.
From

<script src="../plugins/slick.cellcopymanager.js"></script>

// code
var copyManager = new Slick.CellCopyManager();`

To

<script src="../plugins/slick.cellexternalcopymanager.js"></script>

// code
var copyManager = new Slick.CellExternalCopyManager();

@6pac
Copy link
Owner

6pac commented May 17, 2018

Sorry, should have given more info. example-spreadsheet is a single-cell copy and paste. I found it works with the reverted version, but not with the new version.

I found the copy paste was broken, and also I was getting the range designators (eg. A3:B6) pasted into cells. That may be a separate issue, but it didn't happen in the reverted version. That said, I'm not sure what was triggering it to occur.

@ghiscoding
Copy link
Collaborator

ghiscoding commented May 17, 2018

My PR only has 2 lines of code, it should be easy enough to find which line. But again, if I revert my code manually I don't see it working still

EDIT
Ah wait, I didn't know this Copy and Paste was only working in the same grid, not outside like the other plugin. The issue seem to be caused by the newly added last argument at this line

The new argument suppressActiveCellChangedEvent that I added was explained in my PR.

What was happening is that the Editor was getting triggered but canceled out by the row selection. The fix is to set the setActiveCellInternal function 4th argument to True (that arg is suppressActiveCellChangedEvent). This makes the Editor to work correctly and doesn't trigger the cell active change. Also we want to do this only when our Grid is Editable.

The copy cell plugin probably rely on the on cell changed then, does it?

@6pac
Copy link
Owner

6pac commented May 17, 2018

Might add that to the example documentation. I'm not saying this can be fixed - it might be possible that the two use cases are mutually incompatible. If reverting it would only be a case of 'the devil you know'. We'd need to document that to get the editors working, a patch was necessary. Ugly, but until the time or $$ show up to fix the active cell logic, it may be the only way.

Unless, of course, you can fix this. I'm just not 100% sure this is going to be the only thing broken by that change, though. As I said, I spent about two hours looking at #68 originally, and just found it got more complex the more I looked into it.

@ghiscoding
Copy link
Collaborator

ghiscoding commented May 17, 2018

Well I need the PR #68 to get Row Selection and Editors to work together, without it, it just doesn't work in my project. However, if you want a quick and dirty way to fix this, I suggest to add a grid option flag that we could use to enable that 4th argument suppressActiveCellChangedEvent that I added to the setActiveCellInternal function. You could have that flag disable by default and I would enable it to get my code working with Row Selection and Editor.

Thinking of a name for the flag, maybe suppressActiveCellChangeOnEdit which would be false by default.

@ghiscoding
Copy link
Collaborator

I think we can close it now since we addressed the issue in latest PR

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

No branches or pull requests

3 participants