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

Knob: hide cursor on wheel event for .8s #11077

Merged
merged 4 commits into from
Dec 8, 2022

Conversation

ronso0
Copy link
Member

@ronso0 ronso0 commented Nov 18, 2022

While working on #11041 I got bothered by the mouse pointer covering the effect parameter value when using the mouse wheel to turn the knob.

This hides the pointer like for mouse move (drag) events. Atm it's useful only for effect parameter knobs, but for consistency it's applied to all knobs.
Timeout is .8 seconds, the same time the parameter value is shown

@github-actions github-actions bot added the ui label Nov 18, 2022
@daschuer
Copy link
Member

Good idea.

This kind of confirms my concerns of changing the mouspointer to a hand.

@daschuer
Copy link
Member

Is only the pointer hidden or is the mouse kind of catched like when turning the knob klick+move
For my understanding we want the later?

@ronso0
Copy link
Member Author

ronso0 commented Nov 18, 2022

This kind of confirms my concerns of changing the mouspointer to a hand.

Your comment confirms my impression you've been mixing things up ; )
This is about the knob, the hand cursor is for the parameter label only.

Is only the pointer hidden or is the mouse kind of catched like when turning the knob klick+move
For my understanding we want the later?

Why would we want to lock the cursor position when scrolling? If you move the cursor while scrolling, that's okay, as soon as the cursor leaves the widget, the regular cursor is restored so it can always be located (unlike hiding it during drag events).
(this reminds me to unset the cursor in LeaveEvent so it can't be hidden by hovering the knob during the timeout (when not scrolling))

@daschuer
Copy link
Member

Your comment confirms my impression you've been mixing things up ; )

Yes, you are right unfortunately.
I think I need to test it tonight.

I still do not understand how this solves the problem. When you move the mouse during scrolling and hit another widget it is hovered.
That is distracting.

But before hovering another widget you leave the knob widget which bypassed the solution here.
Is this an issue?

@ronso0
Copy link
Member Author

ronso0 commented Nov 18, 2022

(this reminds me to unset the cursor in LeaveEvent so it can't be hidden by hovering the knob during the timeout (when not scrolling))

done.

I think I'll also may implement lazy timer creation like for ControlPushButtonBehavior since the same reasoning applies to knobs, too.

// We create many hundreds of push buttons at Mixxx startup and most of them
// never use their timer. Delay creation of the timer until it's needed.
QTimer* getTimer() {
if (m_pushTimer.isNull()) {
m_pushTimer.reset(new QTimer());
}
return m_pushTimer.data();
}

@ronso0 ronso0 marked this pull request as ready for review November 18, 2022 19:51
@ronso0
Copy link
Member Author

ronso0 commented Nov 18, 2022

When you move the mouse during scrolling and hit another widget it is hovered.
That is distracting.

I don't understand, please describe the distraction in that case.
Do you think users move the mouse wider than a knob widget while scrolling like mad?

Btw I think move events could of course be ignored while scrolling, though not while the cursor is still hidden some time after scrolling, because then I'd expect the cursor move again (like instant release after a drag event).

@daschuer
Copy link
Member

I have just tested this PR and it works like described. Now I also understand the issue and how it is solved.

@ronso0 ronso0 force-pushed the knob-wheel-blank-cursor branch 2 times, most recently from 0d8988c to f359e4a Compare November 22, 2022 20:55
@ronso0
Copy link
Member Author

ronso0 commented Dec 4, 2022

ping @daschuer

Copy link
Member

@daschuer daschuer left a comment

Choose a reason for hiding this comment

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

Sorry for the delay. A few more findings.

src/widget/knobeventhandler.h Outdated Show resolved Hide resolved
src/widget/knobeventhandler.h Outdated Show resolved Hide resolved
src/widget/knobeventhandler.h Outdated Show resolved Hide resolved
Copy link
Member

@daschuer daschuer left a comment

Choose a reason for hiding this comment

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

Thank you. Looks and works good.

@daschuer daschuer merged commit 1edfbde into mixxxdj:main Dec 8, 2022
@ronso0 ronso0 deleted the knob-wheel-blank-cursor branch December 8, 2022 08:34
@ronso0 ronso0 added the changelog This PR should be included in the changelog label Dec 9, 2022
@ronso0 ronso0 added this to the 2.4.0 milestone Dec 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog This PR should be included in the changelog ui
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants