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

Tue Dec 18 07:29:23 GMT 2012

> 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.

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?

- Frank

-----------------------------------------------------------
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/20121218/67ec6135/attachment.htm>