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

feat(core): add new preventDragFromKeys grid option, fixes #1537 #1538

Merged
merged 3 commits into from
May 23, 2024

Conversation

ghiscoding
Copy link
Owner

  • the issue was brought in discussion The row selection is interrupted randomly #1537, the root cause was the mouse drag sometime kicks in when the user selects a few row by using the Ctrl+click combo, however in rare occasion the user might move by even a single pixel and that sends an onDrag event which the SlickCellRangeSelector picks up when then sends a new event onCellRangeSelecting and then the SlickRowSelectionModel assumes it's a new range from a mouse drag and override the previous range. However we should really prevent this mouse drag from happening when the user is pressing the Ctrl/Shift keys to avoid having the issue brought in The row selection is interrupted randomly #1537

- the issue was brought in discussion #1537, the root cause was the mouse drag sometime kicks in when the user selects a few row by using the Ctrl+click combo, however in rare occasion the user might move by even a single pixel and that sends an onDrag event which the SlickCellRangeSelector picks up when then sends a new event `onCellRangeSelecting` and then the SlickRowSelectionModel assumes it's a new range from a mouse drag and override the previous range. However we should really prevent this mouse drag from happening when the user is pressing the Ctrl/Shift keys to avoid having the issue brought in #1537
Copy link

stackblitz bot commented May 22, 2024

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

@ghiscoding
Copy link
Owner Author

ghiscoding commented May 22, 2024

@zewa666 I wouldn't mind a review on this PR since I know you use this feature too and want to make sure it's safe to do so. See #1537 for more info. I'm not sure if the name preventDragFromKeys is meaningful enough though, however I do want to keep it short (with time I realized that super long prop name are never minimized, so better keep it short)

Side note, it turns out that the over the top answer I provided to the SO with SQL LIKE, was so happy that he sent me a bunch of Ko-Fi ☕, so I'm happy too 😉

Copy link

codecov bot commented May 22, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.8%. Comparing base (a2799f2) to head (7f5223b).
Report is 1 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff            @@
##           master   #1538     +/-   ##
========================================
+ Coverage    99.8%   99.8%   +0.1%     
========================================
  Files         198     198             
  Lines       21661   21665      +4     
  Branches     6965    7240    +275     
========================================
+ Hits        21600   21604      +4     
  Misses         61      61             

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ghiscoding
Copy link
Owner Author

I might need to change the keys to only prevent drag on Ctrl key and leave Shift as allowed to drag. Since that seems to cause friction by the user in the discussion

@zewa666
Copy link
Contributor

zewa666 commented May 22, 2024

I've tried out the stackblitz PR review and did so for both Example 17 and Example 19 where I had somewhat similar issues. Both work fine. I couldn't replicate the shift behavior as mentioned in the other discussion but holding shift and clicking around in general causes weird text selections to happen so I didn't give that much thought anyways. But since it's configurable I don't see any big issues with not including it as default since the consumer can always decide which keys to take

@ghiscoding ghiscoding merged commit 803fbee into master May 23, 2024
6 checks passed
@ghiscoding ghiscoding deleted the feat/prevent-drag-keys branch May 23, 2024 19:39
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

Successfully merging this pull request may close these issues.

None yet

3 participants