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.

Sign up for GitHub

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

Jump to bottom

fix(common): Excel copy cell ranges shouldn't lose its cell focus #901

Merged
merged 3 commits into from
Feb 16, 2023
Merged

fix(common): Excel copy cell ranges shouldn't lose its cell focus #901

merged 3 commits into from
Feb 16, 2023

Conversation

ghiscoding
Copy link
Owner

@ghiscoding ghiscoding commented Feb 14, 2023 •
edited
Loading

  • when using Excel copy buffer to copy cell ranges, the cell loses its focus after the copy execution, so we need to reapply the focus on the active cell that the user clicked

TODOs

  • requires newer SlickGrid version which includes this PR

@ghiscoding
fix(common): Excel copy cell ranges shouldn't lose its cell focus
b33097b 
- when using Excel copy buffer to copy cell ranges, the cell loses its focus after the copy execution, so we need to reapply the focus on the active cell that the user clicked
@ghiscoding ghiscoding changed the title fix(common): Excel copy cell ranges shouldn't lose its cell focus WIP - fix(common): Excel copy cell ranges shouldn't lose its cell focus Feb 14, 2023
@codecov
Copy link

codecov bot commented Feb 14, 2023 •
edited
Loading

Codecov Report

Merging #901 (990e7b5) into master (eaf4155) will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##            master      #901   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          244       244           
  Lines        16567     16595   +28     
  Branches      5895      5907   +12     
=========================================
+ Hits         16567     16595   +28
Impacted Files Coverage Δ
packages/common/src/services/gridEvent.service.ts 100.00% <100.00%> (ø)
packages/excel-export/src/excelUtils.ts 100.00% <0.00%> (ø)
packages/excel-export/src/excelExport.service.ts 100.00% <0.00%> (ø)
...ckages/common/src/formatters/formatterUtilities.ts 100.00% <0.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@ghiscoding ghiscoding changed the title WIP - fix(common): Excel copy cell ranges shouldn't lose its cell focus fix(common): Excel copy cell ranges shouldn't lose its cell focus Feb 16, 2023
@ghiscoding ghiscoding merged commit 1dc8b76 into master Feb 16, 2023
@ghiscoding ghiscoding deleted the bugfix/copy-cell-ranges branch February 16, 2023 16:40
ghiscoding added a commit that referenced this pull request Feb 23, 2023
@ghiscoding
fix: Edt cell mouseout should save & excel copy buffer should still work
5e32a98 
- 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 ghiscoding mentioned this pull request Feb 23, 2023
Merged
ghiscoding-SE pushed a commit to ghiscoding/Angular-Slickgrid that referenced this pull request Feb 23, 2023
@ghiscoding
fix: Edit cell mouseout should save & excel copy should work, fix #1103
295c81c 
- 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 ghiscoding mentioned this pull request Feb 23, 2023
Merged
1 task
ghiscoding added a commit that referenced this pull request Feb 23, 2023
@ghiscoding
fix: Edt cell mouseout should save & excel copy buffer should still w…
18ba0fc 
…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 pull request Feb 23, 2023
@ghiscoding
fix: regression Edit cell mouseout should save & excel copy should work
687bd41 
- 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 ghiscoding mentioned this pull request Feb 24, 2023
Merged
1 task
ghiscoding added a commit to ghiscoding/aurelia-slickgrid that referenced this pull request Feb 24, 2023
@ghiscoding
fix: regression Edit cell mouseout should save & excel copy should wo…
2fa2761 
…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 pull request Feb 24, 2023
@ghiscoding
fix: regression Edit cell mouseout should save & excel copy should work
90ec610 
- 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 ghiscoding mentioned this pull request Feb 24, 2023
Merged
1 task
ghiscoding added a commit to ghiscoding/slickgrid-react that referenced this pull request Feb 24, 2023
@ghiscoding
fix: regression Edit cell mouseout should save & excel copy should wo…
152bc80 
…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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@ghiscoding