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

Effect parameter label: set hand cursor to make dragability more discoverable #11062

Merged
merged 1 commit into from
Jan 9, 2023

Conversation

ronso0
Copy link
Member

@ronso0 ronso0 commented Nov 12, 2022

No description provided.

@ronso0 ronso0 marked this pull request as ready for review November 12, 2022 22:20
@github-actions github-actions bot added the ui label Nov 12, 2022
@daschuer
Copy link
Member

Thank you, it works as proposed. The CI failure is unrelated.

While I am not against merging this, I am unsure if this is actually an improvement. The rare "sorting the parameter use case" is now very prominent.
Following the "open hand" = grag-able we need to do much more for consistency.
On the other hand having the "open hand" in the whole library table for instance would feel unusual.
Dragging the Deck Title label is also such a hidden feature.

Here on Ubuntu Focal, it is already the closed hand, shown during dragging.

What do others think?

@ronso0
Copy link
Member Author

ronso0 commented Nov 15, 2022

Here on Ubuntu Focal, it is already the closed hand, shown during dragging.

Yes, the behaviour while dragging is already fine.
Seems QDrag::exec() acivates the native drag cursor.

Dragging the Deck Title label is also such a hidden feature.

Yes, but at least that (as well as double-click and right-click) is mentioned in the tooltip.
I think we could test the hand cursor there, too.

I think you perceive the hand cursor for fx params is 'prominent' because currently we don't use context cursors anywhere else. Another candidate is the text cursor for spinboxes and the searchbox.

@daschuer
Copy link
Member

I am still unsure about this. Can anyone else test this and share the opinion?

@ronso0
Copy link
Member Author

ronso0 commented Nov 15, 2022

Just an idea regarding discoverability and maybe "What's new?", dunno if that's overkill:
we could show the hand cursor to make some action discoverable, and when one d'n'd operation succeeded we assume the user learned that, and set a flag to not apply the hand cursor anymore from now on. This could be per context (deck, fx params, ...)

Note I won't fight for this being merged, but IMO it's the best option atm to make this new feature visible. Very few users seem to read the manual, I bet even those won't read it again after an update. So we should add that to the manual, but I doubt it helps much. Squeezing this into every parameter tooltip seems overkill, but maybe we should do that instead, idk.

@daschuer
Copy link
Member

In general I agree that we should make drag and drop more discoverable.
I also fully support the idea to turn the cursor into a hand when hovering for instance the track title.
For the library we can even add a drag handle icon that turns the mouse into a hand.

My hesitations in the case here are sourced by the issue that sorting the parameters is only a one time task.
Later during performace, the mouse pointer will pointlessly move the shape when trying to adjust the parameter knob. This might be a visual distraction.

Is there a way to solve both?
Maybe we should consider to use a config mode or such to enable the feature. ... :-/

@ronso0
Copy link
Member Author

ronso0 commented Nov 15, 2022

Later during performace, the mouse pointer will pointlessly move the shape when trying to adjust the parameter knob. This might be a visual distraction.

I understand the distraction aspect, but the pointer will only change if you hover the parameter name.

@daschuer
Copy link
Member

daschuer commented Jan 8, 2023

Can you rebase this on master. I think this should fix the Qt 6.3 build.
@Swiftb0y Do you like the hand solution? Merge or not?

@Swiftb0y
Copy link
Member

Swiftb0y commented Jan 8, 2023

IMO this is an improvement, though I'm confused why you are asking for my opinion on this particular feature.

@ronso0 ronso0 force-pushed the eff-param-hand-cursor branch from e3c608e to d843207 Compare January 9, 2023 00:01
@daschuer
Copy link
Member

daschuer commented Jan 9, 2023

IMO this is an improvement, though I'm confused why you are asking for my opinion on this particular feature.

Thank you for confirming. I was just looking for a second unbiased opinion.

@daschuer daschuer merged commit 2c751e7 into mixxxdj:main Jan 9, 2023
@ronso0 ronso0 deleted the eff-param-hand-cursor branch January 9, 2023 08:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants