Continuous drag actions and single click delay #46
Conversation
|
==== 1) Continuous ====
This is true only for Xorg session, on wayland there is a check for the drag distance which triggers the action without having to release the mouse, but I couldn't make it work the same on X11 when dragging a window outside of the panel #32 8937a06 So I ended up with two different handlers: property Component pointHandlerComponent: PointHandler {
target: null
cursorShape: (active && dragging) ? Qt.ClosedHandCursor : Qt.ArrowCursor
onActiveChanged: {
if (active) {
dragging = true
startPos = this.parent.mapToGlobal(point.pressPosition.x, point.pressPosition.y)
localStartPos = this.parent.mapFromGlobal(startPos.x, startPos.y)
printLog `Drag start: ${startPos}`
}
}
onPointChanged: {
if (active && dragging) {
endPos = this.parent.mapToGlobal(point.position.x, point.position.y)
const distance = getDistance(startPos, endPos)
if (!tapHandler.pressed && distance >= minDragDistance) {
runDragAction()
}
}
}
}
property Component dragHandlerComponent: DragHandler {
target: null
cursorShape: (active && dragging) ? Qt.ClosedHandCursor : Qt.ArrowCursor
acceptedDevices: PointerDevice.AllDevices
grabPermissions: PointerHandler.ApprovesCancellation
onActiveChanged: {
if (active) {
dragging = true
startPos = dragHandler.parent.mapToGlobal(persistentTranslation.x, persistentTranslation.y)
localStartPos = dragHandler.parent.mapFromGlobal(startPos.x, startPos.y)
printLog `Drag start: ${startPos}`
}
}
onGrabChanged: {
printLog `onGrabChanged`
if (dragging) {
printLog `(active && dragging)`
endPos = dragHandler.parent.mapToGlobal(persistentTranslation.x, persistentTranslation.y)
const distance = getDistance(startPos, endPos)
if (!tapHandler.pressed && distance >= minDragDistance) {
runDragAction()
}
}
}
}
...
Component.onCompleted: {
if (Qt.platform.pluginName.includes("wayland")){
dragHandler = pointHandlerComponent.createObject(root)
} else {
dragHandler = dragHandlerComponent.createObject(root)
}
plasmoid.configuration.screenWidth = horizontal ? screenGeometry.width : screenGeometry.height
}As you can see the feature will need to work for both handlers, but they are pretty similar, what is needed is just removing the check to the pressed state there and not setting
Really like this idea, but it should be disabled by default
The sensitivity issue can be solved by checking the drag distance, we already have the function
I don't like disabling and changing action names in the UI, we already have ==== 2) Touch Friendly ====
Interesting, I don't have a touch screen capable computer and didn't know touch screen mode didn't have a way to emulate these to work like touchpads do. This feature is fine as long as it is disabled by default. ==== 3) Enable Double Click ====
Nice, you just made me figure out a bug I introduced two months ago that made the Disabled, Custom command a Launch Application/URL actions inaccessible from the actions list, fixing that will make this option unnecessary in the UI and instead you can use something like this
As mentioned earlier, is better to keep the options that change current behavior disabled by default
You're welcome and thanks for your interest in improving it! Let me know if you have any questions about these suggestions. |
|
Thanks for the feedback! I agree that I don't want to change any of the defaults. Interesting about the touch handler. I'll have to look into it. And glad to have helped with the bug! I kept seeing references to Command and App/URL, but I couldn't figure out how to access them. Having that would likely remove my desire for Touch Friendly as well as the axis limiting in isContinuous since I can just disable the commands/axes I don't want. Firing on minDragDistance as you described with a configurable minDragDistance would be the correct solution, I agree. |
Pushed 1750297 to main be sure to update your local branch to pull that change |
Try with this and let me know if it works on X11, it works for me on wayland but can't test touchscreen @@ -457,7 +457,6 @@ PlasmoidItem {
function runDragAction() {
btn = ''
- dragging = false
printLog `Drag end: ${endPos}`
const dragDirection = getDragDirection(startPos, endPos)
printLog `Drag direction ${dragDirection}`
@@ -499,8 +498,10 @@ PlasmoidItem {
if (active && dragging) {
endPos = this.parent.mapToGlobal(point.position.x, point.position.y)
const distance = getDistance(startPos, endPos)
- if (!tapHandler.pressed && distance >= minDragDistance) {
+ if ((!tapHandler.pressed || isContinuous) && distance >= minDragDistance) {
runDragAction()
+ startPos = endPos
+ if (!isContinuous) dragging = false
}
}
}
@@ -511,6 +512,9 @@ PlasmoidItem {
cursorShape: (active && dragging) ? Qt.ClosedHandCursor : Qt.ArrowCursor
acceptedDevices: PointerDevice.AllDevices
grabPermissions: PointerHandler.ApprovesCancellation
+ property real activeX: xAxis.activeValue
+ property real activeY: yAxis.activeValue
+ signal pointChanged()
onActiveChanged: {
if (active) {
dragging = true
@@ -520,14 +524,24 @@ PlasmoidItem {
}
}
- onGrabChanged: {
- printLog `onGrabChanged`
+ onActiveXChanged: {
+ pointChanged()
+ }
+
+ onActiveYChanged: {
+ pointChanged()
+ }
+
+ onPointChanged: {
+ printLog `onPointChanged`
if (dragging) {
printLog `(active && dragging)`
endPos = dragHandler.parent.mapToGlobal(persistentTranslation.x, persistentTranslation.y)
const distance = getDistance(startPos, endPos)
- if (!tapHandler.pressed && distance >= minDragDistance) {
+ if ((!tapHandler.pressed || isContinuous) && distance >= minDragDistance) {
runDragAction()
+ startPos = endPos
+ if (!isContinuous) dragging = false
}
}
} |
|
I added a temporary isContinuous boolean set to true in order to test this, but I am happy to report that it works for me both in X11 with a mouse as well as in wayland with a trouchscreen! It does not work with a stylus in wayland though. (According to libinput list-devices, my stylus is listed with "Capabilities: tablet" whereas the touch screen is "Capabilities: touch".) I also temporarily added a smaller minDragDistance (set to 0) because I want something as close to a 1-to-1 gesture as possible. It seems to work as well as it should. And setting it to any other number, including its current behavior of using the panel thickness, also works exactly as expected. |
This is now only a tiny wishlist item, but one benefit of mangling the action names was that I could take a configured horizontal panel and relocate it to a side so that it becomes vertical without needing to change the action configurations. The concept of a "parallel" axis is lost when I have to explicitly set all four cardinal directions. |
This is great! Thanks a ton for this fix. Love being able to set certain actions to be disabled. If you don't mind, while you're at it, could you add the check that you suggested when double click's action is Disabled to speed up single clicks? If so, this covers all of my use cases! <3 |
Hmm I wasn't considering this, would it make sense to separate the continuous drag from this so we have: Continuous drag: Make the drag action repeat with continued dragging, the code above seems to do this just fine, on wayland even allows chaining actions without leaving the mouse (e.g a drag down[triggers action] then lef/right[triggers respective actions]), I think this is cool. Swap drag direction: Swap drag actions depending on the panel vertical/horizontal state. would this cover your desired workflow? |
Yes, having an option to automatically swap the axes when vertical/horizontal changes orientation would definitely work for me! I was briefly thinking about the continuous drag event chaining too. I like the idea! |
Oh -- I just found a bug with the new implementation. It seems to fire the long press action while I am continuously dragging, or possibly when I pause the motion after having done a continuous drag. Obviously I can simply disable the long press action (which I had done before, hence why I didn't notice until just now!), but my plasma session gets horribly stuck when the default Expose All long press was triggered while continuously dragging. |
Sure I can do that over the weekend, if you want to create a new pull request that adds that and the orientation swap is fine too. We can continue to refine the continuous drag feature here. |
Wait wait, this might have been user error on my side -- I had drag down set to the same Expose All shortcut, and it might have been a drag down action that triggered and not the long press while I was testing out left/right continuous drag. (Something about the Expose All kwin effect really doesn't like whatever actions I'm trying to trigger continuously.) |
|
(Oops! I meant to discard a comment that I was in the middle of typing, but instead I managed to close the entire pull request!)
Do you think the best way to do this is to add a change listener to the horizontal property and then reassign the stored actions in Up<->Right and Down<->Left? |
Yeah that change on the dragHandlerComponent brought back the X11 drag window issue, is the action that triggers the freeze for me, no other action does. Does it happen with other actions for you? If so, this could mean the continuous drag might only be possible on wayland :/ |
It seems more appropriate to do it in getDragDirection function |
I'll have to investigate. My computer was so unhappy that I ran away and didn't look back. |
Won't the UI labels be incorrect if we do the swap in the getDragDirection function? |
On X11 you could switch to a TTY and restart KWin with |
Hmm yes, we can store the horizontal state in a new |
You know, the more I think about this, the less sure I am that I even want such a feature. It makes a lot of sense for the parallel axis, but the swapping logic is unclear to me with the perpendicular axis. If the panel is originally at the top of the screen, then down drags are available easily but upwards ones run into the screen edge. If we move that panel to the right edge, then what used to be down drag would be swapped to left drag. So far so good. But if we move that again to the bottom edge, then the drag direction becomes downward again, but this time the action is impossible because we cannot drag beyond the edge of the screen. This defeats the purpose of the auto swap I think. |
IMHO this is a very niche thing most people won't use because we don't usually move panels around
What I was thinking is at some point was to create presets and preset auto-loading just like I did for https://github.com/luisbocanegra/plasma-panel-colorizer, which would be a lot more flexible:
A horizontal and vertical state could be detected to apply different presets with completely different actions |
Yes, I agree! It's even too niche for me now that I think about it more.
I use panel colorizer all the time! Love it! And I like the presets there. Though I feel like for me personally, I change my mind about aesthetics much more frequently than I do about workflow, so having presets is more obviously useful to me there as opposed to the niche usage here. |
|
On a slightly unrelated note -- when we fire a continuous drag action, we theoretically know the distance that was dragged via the difference between end and start positions. I don't know anything at all about the internals of kwin scripting, but do you know if there is a way to pass the end-start delta magnitude into a shortcut? I'm still thinking about continuous 1:1 drag gestures. It would be neat to provide information about the timing or the speed of the drag. |
AFAIK there is not, we just call using D-bus the same actions a shortcut would call e.g: qdbus6 org.kde.kglobalaccel /component/kwin invokeShortcut "Switch to Next Desktop" |
Ah, I see. Thanks for the clarification. |
I did some more testing, and it seems that long press does NOT get triggered mid drag. The issues I was having were user error on my part. As such, I put together a set of changes that include an isContinuous UI check box along with the associated code using the patch you provided. I also snuck in the allowDoubleClick check that you suggested. :3 Lemme know how this all looks! I'm not super familiar with github and pull requests, so not entirely sure if the latest code made it here correctly. Thanks |
luisbocanegra
force-pushed
the
isContinuous
branch
from
1849c4f
to
3fa93f9
Compare
|
Hi I decided to squash everything into a single commit to keep things cleaner. Now we just need to fix or avoid the X11 freeze maybe we should block the continuous drag when the action is set to move a window? |
The squashed version looks great, as do your wording changes. Thanks! Regarding the Window Move action, I think you're right -- any action that changes the state of the desktop should terminate (or block entirely) continuous dragging. I'm not sure how hard it would be to build a list of such actions, but off the top of my head I can think of Window Move, Resize, Present Windows, Expose, Overview, and Desktop Grid. Certainly there are more too. |
Switched to a single PointHandler, if running on X11 and a drag perpendicular to the panel is detected, skip and run the action when the mouse is released refs: luisbocanegra#32
luisbocanegra
force-pushed
the
isContinuous
branch
from
fa773c0
to
77b0e6e
Compare
Fixed it I think, now if a perpendicular drag is detected we fallback to trigger the action when the mouse is released. Can you test on X11?
Yes, added a check to end the dragging if the action is one of these, hopefully I got them all |
Brilliant! You work very quickly! I just tested out the case that was causing me issues previously -- horizontal panel, continuous drag on left/right, and Expose All on drag down. Happy to report that the Expose All effect only triggered at the end of the drag after mouse up, and the entire time the continuous left/right drags were firing correctly. On initial test, this all looks good to me! |
Tested this in isolation too. Seems to be working! Added a continuous-safe drag action to the perpendicular direction, and indeed it fired only once upon mouse release. |
luisbocanegra
changed the title
|
Thanks for testing, the initial contribution and idea, for me this is ready to be merged. 🎉 Feel free to open another issue if you wish for the parallel drag distance to be customizable and profile/axis switching if you think it is still a good idea or have other ideas for improvement that could be implemented. |
|
Thanks a ton! It was a pleasure working with you. Will definitely keep you posted if I have any other ideas :) |
Hi! Love your project. Wanted to add a couple options to make things slightly better for my use cases. This PR adds 3 new options.
I'll explain briefly what each one does below.
==== 1) Continuous ====
In the traditional behavior of the panel spacer, drag actions are discrete and completed only when the drag is finished. I wanted a continuous action that triggers whenever the pointer is moved while dragging. This new setting allows for this behavior.
When using continuous drag actions, it's very sensitive to small motions, so for now I forcefully allowed only the axis of motion that is parallel to the panel. When you toggle the new Continuous option, the Left/Right drag actions are disabled for vertical panels and the Up/Down drag actions are disabled for horizontal panels. The corresponding disabled config UI elements are hidden as well.
==== 2) Touch Friendly ====
On a touch screen, I do not have middle click or mouse wheel, so this option simply disables those actions and removes them from the config UI.
==== 3) Enable Double Click ====
There is traditionally a 300ms timer on single clicks while waiting for a potential double click. I found this to feel very sluggish in cases when I wanted to repeatedly single click. Disabling the Double Click checkbox reduces the double click watch timer to a very small value and disables the relevant config UI.
====
When Continuous is set to False, Enable Double Click is set to True, and Touch Friendly is set to False, the behavior of the spacer with the proposed changes is identical to the traditional behavior without the PR.
Thanks again for the great widget! Lemme know if you like any of these changes.