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

fix: getCellFromPoint() should return row/cell -1 outside grid canvas #1325

Merged
merged 6 commits into from
Jan 15, 2024

Conversation

ghiscoding
Copy link
Owner

@ghiscoding ghiscoding commented Jan 12, 2024

  • the 1st approach was to throw an error when outside the grid canvas but that caused multiple errors when using auto-scroll (which is normal to fall outside the canvas to let the auto-scroll do its job). So to avoid all these errors, I opted with -1 instead when outside the canvas.
  • So why not use return null? That would require too much refactoring and potentially bring a breaking change to the users who expect the getCellFromPoint() function to always return an object structured as { cell: a, row: b } (where a and b are numbers), so returning -1 has less impact and is still identifiable by being outside the canvas

cc @pbower

Copy link

codecov bot commented Jan 12, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (c5f6b85) 97.7% compared to head (857fb98) 98.5%.
Report is 9 commits behind head on master.

❗ Current head 857fb98 differs from pull request most recent head 1939cae. Consider uploading reports for the commit 1939cae to get more accurate results

Additional details and impacted files
@@           Coverage Diff            @@
##           master   #1325     +/-   ##
========================================
+ Coverage    97.7%   98.5%   +0.8%     
========================================
  Files         198     198             
  Lines       21285   21272     -13     
  Branches     7093    7095      +2     
========================================
+ Hits        20788   20937    +149     
+ Misses        497     335    -162     

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

@ghiscoding ghiscoding changed the title fix(core): getCellFromPoint() shouldn't return negative row/cell fix: getCellFromPoint() should return row/cell -1 outside grid canvas Jan 15, 2024
@ghiscoding ghiscoding merged commit b483e62 into master Jan 15, 2024
5 checks passed
@ghiscoding ghiscoding deleted the bugfix/get-cell-from-point-negative branch January 15, 2024 16:04
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.

2 participants