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

Block the wheel -> scrollTop/scrollLeft assignment when window is the instance's scrollable container. #10996

Merged
merged 2 commits into from
May 27, 2024

Conversation

jansiegel
Copy link
Member

Context

The problem in handsontable/dev-handsontable#1902 was caused by the dragToScroll logic reading window's scrollTop property, which shouldn't be defined at all.

Before this PR, the wheel event was translated into deltas that were supposed to be assigned to the scrollable container's scrollTop/scrollLeft properties. In cases of window-scrolled instances, it resulted in the window's scrollTop/scrollLeft properties having NaN as a value, which messed up the dragToScroll logic.

After this change, the logic remains the same for container-scrolled instances, but window-scrolled instances ignore that.

How has this been tested?

Added a test case checking if triggering the wheel event on the instance results in attempts to assign scrollTop/scrollLeft properties of window.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature or improvement (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Additional language file or change to the existing one (translations)

Related issue(s):

  1. handsontable/dev-handsontable#1902

Affected project(s):

  • handsontable
  • @handsontable/angular
  • @handsontable/react
  • @handsontable/vue
  • @handsontable/vue3

Checklist:

@jansiegel jansiegel self-assigned this May 24, 2024
@jansiegel jansiegel requested a review from budnix May 24, 2024 13:07
@jansiegel jansiegel marked this pull request as ready for review May 24, 2024 13:07
@jansiegel jansiegel merged commit a97e5da into develop May 27, 2024
21 checks passed
@jansiegel jansiegel deleted the feature/dev-issue-1902 branch May 27, 2024 07:26
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

2 participants