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

Auto edit is not functioning #1103

Closed
5 tasks done
roopeshkurian opened this issue Feb 23, 2023 · 8 comments · Fixed by #1104
Closed
5 tasks done

Auto edit is not functioning #1103

roopeshkurian opened this issue Feb 23, 2023 · 8 comments · Fixed by #1104

Comments

@roopeshkurian
Copy link

Describe the bug

Thank you for providing a wonderful and easy grid system. After upgrading to the latest version I found an issue.
autoEdit functionality is not working.
You can see it from the Editors example page.
https://ghiscoding.github.io/Angular-Slickgrid/#/editor

Reproduction

Please visit https://ghiscoding.github.io/Angular-Slickgrid/#/editor
then click Task 0 under the 'Title, Custom Editor' header. Then try to change the value. Then when we move to another cell old edited value is replaced with the old value.

Expectation

When we type new values and move to another cell, it should retain the newly entered values.

Environment Info

Angular 15.1.5
Angular-Slickgrid 5.5.1
TypeScript 4.9.5
Browser Mozilla 110
OS windows.

Validations

@ghiscoding
Copy link
Owner

I'm not sure what you mean, I can't reproduce what you're described, it seems to be working fine on my side. Perhaps you can provide an animated gif of your problem? I use ShareX on Windows to create animated gif

Note that the main difference with autoCommitEdit is that it was a grid option that only existed in Angular-Slickgrid and it wasn't in the SlickGrid core lib. What I did was to add autoCommitEdit directly in the core lib, in this PR and this commit, now that it's part of the core lib, I no longer need to patch it in Angular-Slickgrid (actually in Slickgrid-Universal) and the patch was that it was navigating to the next bottom cell and I was forcing a re-navigate to the same cell position, now that it's in the core we no longer need to reforce a cell navigation since we aren't moving cell at all anymore when autoCommitEdit is enabled in the core.

brave_brqXtXZ0aK

@roopeshkurian
Copy link
Author

In my local project, it does not work. So I thought of checking the Demo you uploaded. It does work for me there also. Can you please see the below gif? I will try in my code again.

grid

@ghiscoding
Copy link
Owner

I need a way to replicate the issue or else I'm afraid that I can't do much in troubleshooting the issue. Have you tried clearing your cache maybe? Also I assume that you know that the autoCommitEdit isn't enabled by default (neither is in the demo and neither is in the animated gif that you provided, you must click the checkbox "Auto Commit Edit" for it to be enabled).

@roopeshkurian
Copy link
Author

I found that, After changing values, if I use TAB or 'Enter' key the value gets changed. But If I use the mouse to navigate to the next cell value is not updating.

@ghiscoding
Copy link
Owner

ok I see now, I'm able to reproduce and I'll see what I can do in the next few days to fix this since it's quite a bug indeed. Thanks for the reproduction. Strangely it doesn't have that problem in Slickgrid-Universal itself, but I see the same bug in Aurelia-Slickgrid.

ghiscoding added a commit to ghiscoding/slickgrid-universal that referenced this issue Feb 23, 2023
- fixes a regression caused by previous PR #901 and identified in Angular-Slickgrid issue [1103](ghiscoding/Angular-Slickgrid#1103), the previous PR #901 was put in place to fix cell external copy in a bug reported in Slickgrid-React issue [36](ghiscoding/slickgrid-react#36)
- this bug actually came from a very old patch that was put in place via `suppressActiveCellChangeOnEdit: true` in SlickGrid [PR 243](6pac/SlickGrid#243) and this flag was to avoid trigger an event when the active cell changes, this event was being listened by SlickCellSelectionModel and when triggered was then sending another event with the cell ranges that changed, which then sent another event onSelectedRangesChanged which was itself listened by SlickCellExternalCopyManager and when triggered was calling a `grid.focus()` and when it did that, it was making the editor loses its focus (hence the implementation of `suppressActiveCellChangeOnEdit`) which is required by SlickCellExternalCopyManager to be able to copy & paste cell ranges. After investigating, I was able to remove the use of `suppressActiveCellChangeOnEdit` (now disabled globally) by simply calling `grid.focus()` in each editor prior to itself getting its own focus, so at least our focus remains in the grid and our editor no longer loses its focus and we are also able to finally use Editor + CellExternalCopyManager at the same time without conflict anymore
- hopefully this fixes all regressions and it also removes some old hacks (suppressActiveCellChangeOnEdit) that was put in place, in summary, it should be a lot cleaner now
ghiscoding-SE pushed a commit that referenced this issue Feb 23, 2023
- fixes #1103 caused by a regression introduced in Slickgrid-Universal PR [901](ghiscoding/slickgrid-universal#901)
- requires Slickgrid-Universal PR [917](ghiscoding/slickgrid-universal#917) to be merged and released
- the regression came after I wanted to fix another bug which was that making a cell range and Copy+Paste wasn't working, when fixing that bug it caused a new bug (this regression). This PR should fix both of these bugs and remove a very old hack that was introduced with `suppressActiveCellChangeOnEdit` which is no longer required
- added Cypress E2E tests to cover the bug identified in #1103
ghiscoding added a commit to ghiscoding/slickgrid-universal that referenced this issue Feb 23, 2023
…ork (#917)

- fixes a regression caused by previous PR #901 and identified in Angular-Slickgrid issue [1103](ghiscoding/Angular-Slickgrid#1103), the previous PR #901 was put in place to fix cell external copy in a bug reported in Slickgrid-React issue [36](ghiscoding/slickgrid-react#36)
- this bug actually came from a very old patch that was put in place via `suppressActiveCellChangeOnEdit: true` in SlickGrid [PR 243](6pac/SlickGrid#243) and this flag was to avoid trigger an event when the active cell changes, this event was being listened by SlickCellSelectionModel and when triggered was then sending another event with the cell ranges that changed, which then sent another event onSelectedRangesChanged which was itself listened by SlickCellExternalCopyManager and when triggered was calling a `grid.focus()` and when it did that, it was making the editor loses its focus (hence the implementation of `suppressActiveCellChangeOnEdit`) which is required by SlickCellExternalCopyManager to be able to copy & paste cell ranges. After investigating, I was able to remove the use of `suppressActiveCellChangeOnEdit` (now disabled globally) by simply calling `grid.focus()` in each editor prior to itself getting its own focus, so at least our focus remains in the grid and our editor no longer loses its focus and we are also able to finally use Editor + CellExternalCopyManager at the same time without conflict anymore
- hopefully this fixes all regressions and it also removes some old hacks (suppressActiveCellChangeOnEdit) that was put in place, in summary, it should be a lot cleaner now
ghiscoding added a commit to ghiscoding/aurelia-slickgrid that referenced this issue Feb 23, 2023
- fixes a bug identified in Angular-Slickgrid [issue](ghiscoding/Angular-Slickgrid#1103) caused by a regression introduced in Slickgrid-Universal PR [901](ghiscoding/slickgrid-universal#901)
- requires Slickgrid-Universal PR [917](ghiscoding/slickgrid-universal#917) to be merged and released
- the regression came after I wanted to fix another bug which was that making a cell range and Copy+Paste wasn't working, when fixing that bug it caused a new bug (this regression). This PR should fix both of these bugs and remove a very old hack that was introduced with `suppressActiveCellChangeOnEdit` which is no longer required
- added Cypress E2E tests to cover the bug identified in #1103
ghiscoding added a commit to ghiscoding/aurelia-slickgrid that referenced this issue Feb 24, 2023
…rk (#941)

- fixes a bug identified in Angular-Slickgrid [issue](ghiscoding/Angular-Slickgrid#1103) caused by a regression introduced in Slickgrid-Universal PR [901](ghiscoding/slickgrid-universal#901)
- requires Slickgrid-Universal PR [917](ghiscoding/slickgrid-universal#917) to be merged and released
- the regression came after I wanted to fix another bug which was that making a cell range and Copy+Paste wasn't working, when fixing that bug it caused a new bug (this regression). This PR should fix both of these bugs and remove a very old hack that was introduced with `suppressActiveCellChangeOnEdit` which is no longer required
- added Cypress E2E tests to cover the bug identified in #1103
ghiscoding added a commit to ghiscoding/slickgrid-react that referenced this issue Feb 24, 2023
- fixes a bug identified in Angular-Slickgrid [issue](ghiscoding/Angular-Slickgrid#1103) caused by a regression introduced in Slickgrid-Universal PR [901](ghiscoding/slickgrid-universal#901)
- requires Slickgrid-Universal PR [917](ghiscoding/slickgrid-universal#917) to be merged and released
- the regression came after I wanted to fix another bug which was that making a cell range and Copy+Paste wasn't working, when fixing that bug it caused a new bug (this regression). This PR should fix both of these bugs and remove a very old hack that was introduced with `suppressActiveCellChangeOnEdit` which is no longer required
- added Cypress E2E tests to cover the bug identified in Angular-Slickgrid
ghiscoding added a commit to ghiscoding/slickgrid-react that referenced this issue Feb 24, 2023
…rk (#57)

- fixes a bug identified in Angular-Slickgrid [issue](ghiscoding/Angular-Slickgrid#1103) caused by a regression introduced in Slickgrid-Universal PR [901](ghiscoding/slickgrid-universal#901)
- requires Slickgrid-Universal PR [917](ghiscoding/slickgrid-universal#917) to be merged and released
- the regression came after I wanted to fix another bug which was that making a cell range and Copy+Paste wasn't working, when fixing that bug it caused a new bug (this regression). This PR should fix both of these bugs and remove a very old hack that was introduced with `suppressActiveCellChangeOnEdit` which is no longer required
- added Cypress E2E tests to cover the bug identified in Angular-Slickgrid
ghiscoding added a commit that referenced this issue Feb 24, 2023
…essActiveCellChangeOnEdit

fix: Edit cell mouseout should save & excel copy should work, fix #1103
@ghiscoding
Copy link
Owner

ghiscoding commented Feb 24, 2023

@roopeshkurian thanks for reporting this and providing ways to replicate, it was a hard one to fix and it took a lot of troubleshooting and digging. The new release v5.6.0 should resolve all issues, make sure to also update any Slickgrid-Universal dependency you might have. Cheers

@roopeshkurian
Copy link
Author

Thank you so much for solving it. Like always you were so prompt to address this issue and solve it. It was a great help. Once again thank you :-)

@ghiscoding
Copy link
Owner

Welcome thanks for using the lib 😉 🎉

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

Successfully merging a pull request may close this issue.

2 participants