[Panel Config] Scrolling over size button increments size by 2 and shows current thickness
ClosedPublic

Authored by Zren on Jun 26 2017, 1:17 PM.

Details

Summary

The current thickness is also shown when dragging as well.
The number disappears and is replaced with the original text after 1 second.
Tested and works with a touchpad "mousewheel".

Bug: 372364


Bug Link: https://bugs.kde.org/show_bug.cgi?id=372364
Reddit: https://www.reddit.com/r/kde/comments/65wdow/can_we_get_some_support_for_pixel_perfect_panel/

New Video: https://streamable.com/6w9ry

Diff Detail

Repository
R119 Plasma Desktop
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
Zren created this revision.Jun 26 2017, 1:17 PM
Restricted Application added a project: Plasma. · View Herald TranscriptJun 26 2017, 1:17 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript

I like the idea of showing the thickness in the button. My suggestion would be add a Timer which you restart() when you change the panel thickness (you need to try whether in onThicknessChanged is sufficient or doing that explicitly in onWheelEvent and/or onPositionChanged and then bind the text to whether it's running, this way you don't break any bindings but just have it work like magic ;) for example:

Timer {
    id: panelResizeSizeHintTimer
    interval: 1000
}

...

Button {
    text: panelResizeHintTimer.running ? panel.thickness : i18n("Width") // don't forget "Height" for horizontal panels
    ...
    onWheel: {
        // wheel stuff here
        panelResizeHintTimer.restart()
    ...
}

(QtQuick Timer restart will start it if not running and restart it if already running, it also runs only once by default which is what we want here)

desktoppackage/contents/configuration/panelconfiguration/SizeHandle.qml
123

I don't know exactly how MouseEventListener behaves but in MouseArea you get a ton of onWheel events for touchpads (which scroll pixel-precisely), potentially causing you to making your panel enormous accidentally

125

I think there's also a maximum size enforced (half of screenToFollow.geometry), it's somewhat hidden in the complex onPositionChanged code

129

Missing semicolon here and below, also can it ever get 0 anyway?

davidedmundson added inline comments.
desktoppackage/contents/configuration/panelconfiguration/SizeHandle.qml
125

You need to make it by 2.

See desktoppackage/contents/configuration/panelconfiguration/SizeHandle.qml:50:

Zren added a comment.Jun 26 2017, 4:18 PM

Great idea about using a timer in the binding. I can probably keep the onThicknessChanged binding since it does change size when moved from left/right to top/bottom.

As far as I can tell, the wheel.delta is only using the hardcoded jumps of ±120 (on click) (or 240 is scrolled fast enough). It's not triggered with a value of 0.

Scrolling "up" to increase the top panel size (expand downwards) feels a little weird, but it's probably better to be consistent with the left/right/bottom panels than to give it a special case when scrolling in reversed.

desktoppackage/contents/configuration/panelconfiguration/SizeHandle.qml
123

That's a really good test case I hadn't considered. I wonder if Qt::MouseButtons KDeclarativeWheelEvent.buttons is pressed down in those events. I don't have a laptop/trackpad to test with so is there a utility to test trackpads with the mouse?

129
Zren updated this revision to Diff 15888.Jun 26 2017, 4:25 PM

Jump by 2px. Jump by __ clicks. Make sure thickness is even. Hide thickness in button after 1 second.

Zren marked 3 inline comments as done.Jun 26 2017, 4:26 PM
Zren added inline comments.Jan 3 2018, 6:09 PM
desktoppackage/contents/configuration/panelconfiguration/SizeHandle.qml
123

Recently got a Windows Vista era HP laptop running KDE.
I see, the touchpad "mousewheel" sends a ton of events with wheel.delta from ±9 to ±60.

In my test, I actually noticed the opposite effect (it was slow to resize) since we do Math.round(wheel.delta / 120) which can turn into Math.round(30 / 120) = 0. So events with a wheel.delta smaller than 60 were ignored.

Anyways, I always wondered why plasma-pa did this in their mousewheel code:

https://github.com/KDE/plasma-pa/blob/master/applet/contents/ui/main.qml#L197-L210

Should be an easy fix. Just need to add a wheelDelta variable and ignore events when it's wheelDelta <= -120 || 120 <= wheelDelta.

Zren updated this revision to Diff 24808.Jan 6 2018, 5:03 AM
Zren retitled this revision from [Panel Config] Scrolling over size button increments size by 1 and shows current thickness to [Panel Config] Scrolling over size button increments size by 2 and shows current thickness.
Zren edited the summary of this revision. (Show Details)

Support touchpad "mousewheel" which is pixel perfect based off plasma-pa's code.

Now that I'm submitting this... I could probably also use this instead of the while loops...

var deltaThickness = wheelDelta / 120
deltaThickness = deltaThickness < 0 ? Math.ceil(deltaThickness) : Math.floor(deltaThickness)
wheelDelta -= deltaThickness * 120

Meh. The loop pattern is tested an works (in plasma-pa too).

Zren marked an inline comment as done.Jan 6 2018, 5:04 AM
mmustac added a subscriber: mmustac.EditedJan 29 2018, 8:38 PM

What is the current status here? I would love to see this feature coming in the near future.
Another thing that would be great: If we could click on the labels / buttons and they would switch to an editline where we could enter a value by our own and the pandel would change according. Some people - like me - like their panels pixel perfect specially when you have three monitors and want the same size on all three of them.

FWIW, a major Linux reviewer mentioned this as a wishlist item in his recent review of the Plasma 5.12 beta: http://www.ocsmag.com/2018/01/27/plasma-5-12-long-term-sweetness/

"If you wish to change the bottom panel height, you don’t have a field where you can input an exact number. Instead you need to slide the panel up and down. This isn’t the end of the world, but it can be made simpler for those who prefer precise values."

It would be great to knock this out if we can!

Zren added a comment.Jan 30 2018, 4:09 AM

No one approved it so I never merged it in time for the Plasma 5.12 beta. So I guess it'll have to wait for 5.12 to be released before I merge it into the master branch?

Since there aren't any string changes, technically it could still go into the 5.12 branch. Or it could go into master at anytime.

I approve in concept, FWIW. But we should probably get formal approval from @broulik or @davidedmundson first.

davidedmundson accepted this revision.Jan 30 2018, 11:13 AM

Not in 5.12.

I'm not a huge fan, as there's another open bug that the panel should follow the Plasma style font-size based sizing. When we have both, presenting something in px, only to save in some other unit will lead to somewhat quirky behaviour. But Kai seemed happy and the code looks fine, go for it.

Do you have commit access?

This revision is now accepted and ready to land.Jan 30 2018, 11:13 AM
Zren added a comment.Jan 31 2018, 12:12 AM

I do have commit access.

Or it could go into master at anytime.

Does this means I can merge to master now (and it'll appear in Plasma 5.13)?

only to save in some other unit

As so the plan is to serialize in dpi units? Eg: In units of pixels / units.devicePixelRatio instead of pixels? or something like that? Makes sense.

Yep, if you land this on master, it will wind up in 5.13.

Does this means I can merge to master now (and it'll appear in Plasma 5.13)?

Yes and yes.

This revision was automatically updated to reflect the committed changes.