-
Notifications
You must be signed in to change notification settings - Fork 3.3k
fix: BROS-87: Introduce unified zoom behavior for trackpads and mice #7852
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
Conversation
✅ Deploy Preview for label-studio-docs-new-theme canceled.
|
✅ Deploy Preview for heartex-docs canceled.
|
✅ Deploy Preview for label-studio-storybook ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for label-studio-playground ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
/fmt |
yyassi-heartex
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
minor suggestion but other wise looks good
|
|
||
| // Limit the maximum zoom change per event to prevent aggressive zooming | ||
| // This prevents users from accidentally zooming too far with a single wheel event | ||
| const limitedZoomChange = Math.max( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you need to use clamp() here because you don't limit minimum value
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
basically you just need to run it two times, with ±MAX_CHANGE and with MIN/MAX
hlomzik
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
one important change regarding the goal of the PR, and the rest is good
|
/fmt |
Just moving the functionality under a feature flag.
Reason: buggy behavior.