Implement scroll and drag adjustment of values for SpinBox control
ClosedPublic

Authored by ngraham on May 8 2020, 3:15 PM.

Details

Summary

Currently adjusting the value by scroll and click/touch+drag are not implemented.
This patch implements them.

Test Plan

As far as I can tell, the PlasmaComponents3.Spinbox is not actually used anywhere in Plasma, so test with D29535:

Diff Detail

Repository
R242 Plasma Framework (Library)
Branch
implement-spinbox-wheel-adjustment (branched from master)
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 26565
Build 26583: arc lint + arc unit
ngraham created this revision.May 8 2020, 3:15 PM
Restricted Application added a project: Frameworks. · View Herald TranscriptMay 8 2020, 3:15 PM
Restricted Application added a subscriber: kde-frameworks-devel. · View Herald Transcript
ngraham requested review of this revision.May 8 2020, 3:15 PM
ngraham updated this revision to Diff 82274.May 8 2020, 3:17 PM

Mark the value has having been modified after adjusting by scrolling

cblack added a subscriber: cblack.May 8 2020, 3:21 PM
cblack added inline comments.
src/declarativeimports/plasmacomponents3/SpinBox.qml
49

control.value is already a numeric type as control is a SpinBox, so you don't need to parseInt here. control.value =+ control.stepSize

50

I think you would need to explicitly call this on control? control.valueModified().

52

control.value =- control.stepSize

53

control.valueModified().

ngraham edited the test plan for this revision. (Show Details)May 8 2020, 3:34 PM
ngraham updated this revision to Diff 82276.May 8 2020, 3:36 PM
ngraham marked 4 inline comments as done.

More control, less parseInt

ngraham edited the test plan for this revision. (Show Details)May 8 2020, 3:52 PM
ngraham updated this revision to Diff 82311.May 8 2020, 8:24 PM

Implement click-and-drag (along both the X and Y axes) to modify the value

ngraham edited the summary of this revision. (Show Details)May 8 2020, 8:25 PM
ngraham edited the test plan for this revision. (Show Details)
ngraham retitled this revision from Implement wheel/touchpad scrolling for SpinBox control to Implement scroll and drag adjustment of values for SpinBox control.May 8 2020, 8:37 PM
ngraham edited the summary of this revision. (Show Details)
ngraham edited the test plan for this revision. (Show Details)
ndavis added a subscriber: ndavis.EditedMay 8 2020, 8:43 PM

I don't like the look of those +/- buttons. I think this makes it harder to know that you can drag the panel height up and down.

I don't like the look of those +/- buttons. I think this makes it harder to know that you can drag the panel height up and down.

That would be a comment for D29535, but I will repeat my argument here: I think that drag is the least important interaction method for panel thickness adjustment since you typically want to make fine adjustments or arrive at a specific value, both of which are unsuited for drag-based interactions. Nevertheless, I have implemented drag-adjustment in the spinbox control here to not lose it, which also yields the advantage that other potential Plasma components that use a spinbox will benefit from it.

We can improve the look of this control in another patch. To my knowledge it hasn't been used at all so it wouldn't surprise me if it's a bit unpolished.

abetts added a subscriber: abetts.May 8 2020, 8:48 PM

+1 from me!

ahiemstra added inline comments.
src/declarativeimports/plasmacomponents3/SpinBox.qml
51

You can do mouse.accepted = false instead of forceActiveFocus to allow the mouse press to go to items below it. However, you might instead want to use DragHandler's onActiveChanged for this, as that signals when the actual drag is active.

ngraham updated this revision to Diff 82786.May 13 2020, 7:11 PM
ngraham marked an inline comment as done.

Address review comment

ahiemstra accepted this revision.May 20 2020, 9:30 AM
This revision is now accepted and ready to land.May 20 2020, 9:30 AM
ngraham closed this revision.May 20 2020, 1:38 PM