Review Request: [Dolphin] Fake click for short drag and drop on same item

[Thomas Lübking]

> On Dec. 14, 2012, 3:56 p.m., Frank Reininghaus wrote:
> > First of all, thanks for the patch! I see now that dragging items accidentally when trying to click happens a lot for some users, and I agree that fixing this would be nice.
> > 
> > I'm just wondering if there is an easier way to do it, without making DragAndDropHelper depend on KItemListController and adding the hand-made press/release events. I have two ideas at the moment:
> > 
> > 1. Make KItemListController finally respect http://qt-project.org/doc/qt-4.8/qapplication.html#startDragTime-prop, and only start a drag in KItemListController::mouseMoveEvent() if the time since the mouse press event is larger than that, or
> > 
> > 2. Check in KItemListController::mouseMoveEvent() if the mouse cursor is still above the item where the press happened, and do not start a drag if that is the case.
> > 
> > Both approaches would prevent that the drag is started, whereas your idea was to sort of cancel the drag after it happened. Not starting the drag in the first place might be a bit cleaner and require less code from my point of view. But maybe there is a good reason why you chose to do it in a different way?
> 
> Thomas Lübking wrote:
>     Supporting the dragtimeout is even simpler, but the default of 500ms makes it appear quite laggy on inteded DnD ops (you press, hold, drag and for the first half second nothing happens) - esp. since apparently no Q/KItemView seems to care much about it.
>     
>     If you'd advice ppl. to shorten the timeout to sth. "reasonable" (below 150ms), they'll again run into the accidental drags.
>     
>     I'll update the patch later on to simply respect the timeout so that you can try for yourself.
> 
> Christoph Feck wrote:
>     > you press, hold, drag and for the first half second nothing happens
>     
>     Unless you drag it far enough (= startDragDistance), this is the expected (and correct) behaviour. Try window dragging by click-move on the title bar.
> 
> Thomas Lübking wrote:
>     Ok, thanks (never waited so log ;-)
>     Just that the expected "autodrag w/o move" won't help on the particular issue (since the drag distance is respected and effectively the problem)
>     
>     @Frank: we could simply keep the dragTime in the DragAndDropHelper instead to avoid deps. on the controller.
> 
> Kai Uwe Broulik wrote:
>     If we'd just turn off that "A folder cannot be dropped onto itself" message, a huge annoyance would be already gone ;) *scnr*
> 
> Frank Reininghaus wrote:
>     Thanks Thomas for the new patch and everyone else for the comments, in particular Christoph - I somehow thought the idea was to start a drag when "distance > startDragDistance" && "time since press > startDragTime", but with && replaced by ||, it makes much more sense indeed :-)
>     
>     @Kai: "If we'd just turn off that "A folder cannot be dropped onto itself" message, a huge annoyance would be already gone":
>     
>     No, in that case, we would get bug reports like "Clicking items fails randomly".
>     
>     @Thomas: I see that it's getting better :-) But I still think that this is a lot more complex than it needs to be. Why not just put the timer into KItemListController, start it when the mouse button is pressed, and in mouseMoveEvent, if the drag distance is too small, check if the time since the press is larger than startDragTime and do not start a drag if that is not the case.
>     
>     This should mostly yield the same result and require a lot less code and no dependencies between KItemListController and DragAndDropHelper, or am I overlooking anything?
> 
> Thomas Lübking wrote:
>     This will effectively increase the QApplication::startDragDistance(), because for the first 500ms -assuming default value in place- the drag distance is increased to not "too small" and 500ms is much time in this context.
>     
>     You can try the outcome by increasing it legally in "kcmshell4 mouse" - it's capped @ 20px, though, while a quick judder (the typical movement is down and up)  can be much more (esp. depending on screen resolution and mouse acceleration)
>     
>     I'm willing add an update, but notice that the approach omits the best variable for the heuristic (button release, ie. drag time) and trap us between false positives (dialog/notification) and laggy DnD behavior (even with 20px it's rather "strange" when the DnD suddenly starts)
>     
>     If you just don't like the interdependency, it would be possible to (from the controller) store the msecsSinceReference (kinda "current time", but monotic) as dynamic property "DolphinDragStartTime" on the QApplication object and fetch and compare it from there DnDHelper.
> 
> Frank Reininghaus wrote:
>     Well, I'm not a usability expert, but IMHO, not starting the drag if we think that the user does not want to drag might be better and less distracting than starting the drag, changing the mouse cursor for that, and then reverting the drag after the drop.
>     
>     Code-wise, I also think that not starting the drag is better. It requires a lot less code. Moreover, I'm worried that the hand-made press and release events might lead to strange bugs in the future. They are delivered inside the nested event loop of the drag, and I've really had a lot of unpleasant experiences with nested event loops in the past.
>     
>     I see that replacing the check "distance > startDragDistance" by "distance > startDragDistance || time > startDragTime" would not resolve the "accidental drag" problem (it might make our behaviour more 'correct' though, but I just checked the source of QAbstractItemView and found that it does not care about the startDragTime either).
>     
>     What about the following idea: I guess that users never drag an item to drop it on itself. We could (in KItemListController::mouseMoveEvent) determine the 'index' which belongs to the position of the event and just not start the drag if this index is equal to m_pressedIndex. I've suggested that earlier, but nobody commented on that idea. Am I overlooking anything which is obviously wrong with that idea?
>
> 
> Thomas Lübking wrote:
>     > not starting the drag if we think that the user does not want to drag might be better
>     Yes, of course. The tricky part is to figure what the user wants.
>     The pre-action information is the drag distance, which you intend to increase (by tests in the former and last comments) what's (given the misclick isn't a dolphin only phenomenon) equal to telling the user to do so in the settings.
>     This is oc. the best solution to prefer clicks, but comes with a pubishment to DnD.
>     
>     If you simply increase it internally, be better prepared for "dolpin doen not respect dnd startdistance" bugs.
>     
>     The startdistance actually exists for this purpose (smoothing away dirty clicks) and if usually sufficient, it's not reasonable to increase the setting, nor hardcode a higher value (just think of 128px icons...)
>     
>     What is special about dolphin and source of the bug reports is its treatment of user failure.
>     Usually, if a DnD cannot take place, the action silently quits (previously hinted by the "forbidden" cursor) but dolphin currently "annoys" the user with a (ui re-layouting) message (in this case false positive hint) to "punish" his failure (thus Kais scnr)
>     
>     So my focus would not be on blocking the drag in the first place (there's a setting for this and dolphin does not differ from any other client in that regard) but on failure treatment.
>     
>     This could also be a timeout on just showing the dialog - i just thought that since we pretty much (now!) know what the user intended, we could just do that for him.
> 
> Frank Reininghaus wrote:
>     Well, either I don't understand you, or you don't understand me, but the result is the same either way ;-)
>     
>     I did not say that I intend to increase the 'start drag disctance'. Therefore, I don't see why we would get "dolpin does not respect dnd startdistance" bugs. I probably wasn't clear enough the first two times, so I will just try to say again what my idea to resolve this is:
>     
>     1. When the mouse moves, we check if the mouse cursor is still above the item where the button was pressed. If that is the case, we don't start a drag (just like if the distance between the press and move events is less than the 'start drag distance').
>     
>     2. Because no drag was started, mouseReleaseEvent will behave just like after a click, and will trigger the item.
>     
>     This means: a drag is only started when the cursor moves out of the bounds of the item where the mouse was pressed AND if the distance between press and release is larger than the 'start drag distance', whatever the user chose for it. 
>     
>     Before we continue the discussion, could please anyone tell me what is wrong with this approach?
>     
>     About the 'folder cannot be dropped on itself' message: I didn't invent it, and from my point of view it's fine to replace it with something else, or even to show a 'forbidden cursor' (unless there was a good reason why the message has been added in the first place, which I don't know at the moment).

Sorry, i was abroad (need to move reviewboard to gmail)

> 1. When the mouse moves, we check if the mouse cursor is still above the item where the button was pressed. If that is the case, we don't start a drag

What effectively increases the startDragDistance.

The default value here is 4px while the minimum icon size is 16px, many users will use up to 64px (or even more, eventually plus 2-3 lines of text) what means a startDragDistance of 8px - 32px (if you click the center and move straight out).

I do not discuss in terms of wrong or right (outside math ;-) but there's a notable difference in the behavior if you enforce to leave an item to start the DnD which grows with the icon size.
Assumption that there'll be a bug report about it is just "bugzilla experience gut feeling" - i've no proof for that =)


- Thomas


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/107708/#review23469
-----------------------------------------------------------


On Dec. 14, 2012, 9:25 p.m., Thomas Lübking wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/107708/
> -----------------------------------------------------------
> 
> (Updated Dec. 14, 2012, 9:25 p.m.)
> 
> 
> Review request for Dolphin and Frank Reininghaus.
> 
> 
> Description
> -------
> 
> When there's a relatively short click (i picked 500ms) and the item is moved to DnD and released on itself, this is now assumed a "dirty click" (ie. the user actually wanted to click, but juddered) instead of presenting a warning that an item cannot be dragged on itself.
> 
> Notes:
> - 500ms are quite some time. I can drag the icon out, back in and drop it in place.
> - due to the high abstraction level in the DnD processing and the application window being the drag source, it is technically possible to split the view and DnD an icon onto its other self within 500ms
> 
> I'm however willing to state that if you manage to do either of that, you should not have major issues on performing a regular click either.
> I picked the 500ms on personal test (started with 1500, what seems far too much)
> 
> - the reason for having the timeout in the first place is the assumption, that users may actually intentionally try to drag an item on itself. Either because they intend to link it there (link recursion can be dangerous, but is a legal action) or for "ummm... i didn't want to copy that folder. errr... how do i stop this ... ok, let's just put it back from where it came and hope for the best". Because of the latter i think this should be hinted after the message freeze.
> 
> - one might want to add a "don't ask again" checkbox to the hint and account that by dropping the timeout
> 
> 
> This addresses bugs 283646, 297414, 307747, and 311483.
>     http://bugs.kde.org/show_bug.cgi?id=283646
>     http://bugs.kde.org/show_bug.cgi?id=297414
>     http://bugs.kde.org/show_bug.cgi?id=307747
>     http://bugs.kde.org/show_bug.cgi?id=311483
> 
> 
> Diffs
> -----
> 
>   dolphin/src/kitemviews/kitemlistcontroller.cpp 697e04f 
>   dolphin/src/views/draganddrophelper.h ac16f7c 
>   dolphin/src/views/draganddrophelper.cpp f81d4d0 
> 
> Diff: http://git.reviewboard.kde.org/r/107708/diff/
> 
> 
> Testing
> -------
> 
> clickdragged a folder in both view (w/ and w/o scroll offset)
> 
> 
> Thanks,
> 
> Thomas Lübking
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.kde.org/mailman/private/kfm-devel/attachments/20121221/f7c6c6b1/attachment.htm>