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

Timeline: mouse handling improvements #606

Merged
merged 3 commits into from
Jul 2, 2019

Conversation

rpallai
Copy link
Contributor

@rpallai rpallai commented Jun 27, 2019

No description provided.

An easy-going vertical wheel scroll over the timeline is expected by
users even if shuttleDial is enabled.
Copy link
Member

@KitsuneRal KitsuneRal left a comment

Choose a reason for hiding this comment

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

First of all - many thanks for attacking this topic - Quaternion admittedly sucks at handling text selection, and scrolling still has some room for improvement. I'm not entirely happy with the number of newly introduced objects but I understand that the current model of pointer handling in Qt doesn't leave many other options. I'll give this a ride for a day or two, and if I don't stumble over serious issues I'll merge it.

Copy link
Member

@KitsuneRal KitsuneRal left a comment

Choose a reason for hiding this comment

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

Unfortunately the selection part is not portable: "Support for QClipboard::Selection is provided only on systems with a global mouse selection (e.g. X11)." (from Qt docs). As a result, at least on Windows, just selecting the text immediately (right after releasing the mouse button) leads to two unexpected things:

  • the text gets unselected; if I wasn't careful enough and selected too much or too little, I have to reselect it;
  • the selection goes to the clipboard right away, potentially overwriting whatever I have there.

@rpallai rpallai force-pushed the mousehandling branch 2 times, most recently from c4940ac to 6b23716 Compare June 30, 2019 21:09
@rpallai
Copy link
Contributor Author

rpallai commented Jun 30, 2019

I reworked text selection, now the selected text:

  1. remains selected after selection ended (as before)
  2. immediately copied into global mouse selection buffer (as before)
  3. could be copied to the clipboard with ctrl+c (as before)
  4. gets unselected when new selection starts (new feature)

The only feature I have to reimplement is correction of the selection by shift + click. To tell you truth I wasn't aware of this capability until you had mentioned it.

Dropped files will be attached, dropped text will appear in the
input line.
@rpallai
Copy link
Contributor Author

rpallai commented Jun 30, 2019

The only feature I have to reimplement is correction of the selection by shift + click.

It's also done. Please test.

Copy link
Member

@KitsuneRal KitsuneRal left a comment

Choose a reason for hiding this comment

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

I hope you understand that the day when people come back and ask to support keyboard-based selection in the timeline, you'll get into an even deeper rabbit hole :) Meanwhile - looks like it almost behaves now, just a couple of nitpicks.

client/kchatedit.cpp Outdated Show resolved Hide resolved
client/chatroomwidget.cpp Show resolved Hide resolved
client/qml/TimelineTextEditSelector.qml Show resolved Hide resolved
@KitsuneRal KitsuneRal added the enhancement A feature or change request for Quaternion label Jul 1, 2019
client/kchatedit.cpp Outdated Show resolved Hide resolved
There is no doubt that the most clicked widget is the timeline. We
click it to open pictures, to bring the window forward, to select
text, etc. Unfortunately, these actions ended up with unfocused
input line which was my primary reason to go ape on Quaternion.

This patch introduces a strict focus policy on the timeline which
means the timeline won't keep the focus for long, it will hand back
to the input line when possible, when pressed mouse buttons are
released.

At the same time we had to implement our own TextEdit text
selection mechanism because the original didn't pass through
required mouse events. Added bonus that deselection now works for
different messages (closes quotient-im#300).
@KitsuneRal KitsuneRal merged commit f2a60b8 into quotient-im:master Jul 2, 2019
@KitsuneRal
Copy link
Member

Turned out I was a bit hasty. Your code prevents hyperlinks from working.

@KitsuneRal
Copy link
Member

Also, after spending some more time I now see that the original scrolling is actually about 3 times more precise (it's also slower but not as much) at least on my setup. A case for adjustable sensitivity is quite prominent.

@rpallai
Copy link
Contributor Author

rpallai commented Jul 2, 2019

Also, after spending some more time I now see that the original scrolling is actually about 3 times more precise (it's also slower but not as much) at least on my setup. A case for adjustable sensitivity is quite prominent.

What do you mean under more precise scrolling exactly? Wasn't your jump length constant per every each wheelEvent, so non-linear? Perhaps wheel.pixelDelta is supported by your system?
On my Linux, the original scrolling speed was about 16px / wheelEvent constantly which has been changed to 100px / wheelEvent by this patch (it's 6 times faster now).

Anyway, adjustable sensitivity is indeed important, I'll push that into the preferences window as it will be available (#605).

@KitsuneRal
Copy link
Member

That's the contention point: for me, 16px/notch is almost good enough (maybe a bit slowish). So it's really just about different tastes.

@rpallai
Copy link
Contributor Author

rpallai commented Jul 2, 2019

Oh, I overcomplicated this issue, I thought "sensitivity" is non-linear to "speed" - thanks for clarification. I noticed fa8ded0 restored your speed preference. It's okay, I'll make it configurable as #605 settles down. The hard part is done by now. 🎊

@KitsuneRal
Copy link
Member

Ouch, I didn't really mean to restore it so unceremoniously :( And of course let's make it configurable after merging #605.

With respect to sensitivity - we can make it non-linear but it's yet another Pandora's box. From the UX perspective - very few people would even bother so much to precisely tune it. What I would introduce instead is adding the shuttle-dial style of treating wheel events - with notches controlling the speed rather than distance. That would effectively enable auto-scrolling (imagine speed-reading your timeline without having to constantly turn the wheel to show the next piece).

@KitsuneRal KitsuneRal mentioned this pull request Aug 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement A feature or change request for Quaternion
Projects
Status: Version 0.0.95 - Done
Development

Successfully merging this pull request may close these issues.

2 participants