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

NumericWidget : Mouse wheel adjust value #6045

Merged

Conversation

ericmehl
Copy link
Collaborator

This adds the ability to change a numeric widget value with the mouse wheel in the same manner as the keyboard up and down keys currently do.

I arrived at the scale factor of 15.0 to translate the wheelRotation value to increment units by seeing what the wheelRotation value was for a single "tick" on my mouse. I don't know how universal that is, so could definitely do with some testing by others to see if it changes values in a reasonable way.

Fixes #6009.

Checklist

  • I have read the contribution guidelines.
  • I have updated the documentation, if applicable.
  • I have tested my change(s) in the test suite, and added new test cases where necessary.
  • My code follows the Gaffer project's prevailing coding style and conventions.

Copy link
Contributor

@murraystevenson murraystevenson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Eric! The scale factor of 15 feels about right for my Logitech mouse on Linux, I see roughly 1 (+/- 0.125) increment's worth of change for each "step" of the wheel. I've added a couple of thoughts inline based on some early testing.

python/GafferUI/NumericWidget.py Show resolved Hide resolved
python/GafferUI/NumericWidget.py Outdated Show resolved Hide resolved
Changes.md Outdated Show resolved Hide resolved
@ericmehl
Copy link
Collaborator Author

Thanks @murraystevenson and @johnhaddon! I addressed the comments and also tested on a laptop track pad. It may be a touch too fast using a track pad, but I was still able to increment one at a time without much trouble. Since the mouse case feels natural, I'm inclined to leave it at the current rate.

@ericmehl
Copy link
Collaborator Author

The latest push refines the increment logic and also fixes the scroll signal leaking. In cb7e934 I added to the tooltip to try and make this a little more discoverable.

Ready for more testing and review!

@ericmehl
Copy link
Collaborator Author

I fixed the wording of the tooltip and squashed everything down to one commit. Hopefully ready to merge!

@murraystevenson
Copy link
Contributor

Thanks Eric! Merging.

@murraystevenson murraystevenson merged commit 4d698f1 into GafferHQ:1.4_maintenance Sep 23, 2024
6 checks passed
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.

3 participants