[prev in list] [next in list] [prev in thread] [next in thread]
List: kfm-devel
Subject: Re: Review Request: [Dolphin] Fake click for short drag and drop on same item
From: "Kai Uwe Broulik" <kde () privat ! broulik ! de>
Date: 2012-12-14 18:52:30
Message-ID: 20121214185230.17620.71739 () vidsolbach ! de
[Download RAW message or body]
> 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.
If we'd just turn off that "A folder cannot be dropped onto itself" message, a huge \
annoyance would be already gone ;) *scnr*
- Kai Uwe
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/107708/#review23469
-----------------------------------------------------------
On Dec. 13, 2012, 9:10 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. 13, 2012, 9:10 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.h 235e4a9
> dolphin/src/kitemviews/kitemlistcontroller.cpp 697e04f
> 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
>
>
[Attachment #3 (text/html)]
<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 \
solid;"> <tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="http://git.reviewboard.kde.org/r/107708/">http://git.reviewboard.kde.org/r/107708/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: \
10px;"> <p style="margin-top: 0;">On December 14th, 2012, 3:56 p.m., <b>Frank \
Reininghaus</b> wrote:</p> <blockquote style="margin-left: 1em; border-left: 2px \
solid #d0d0d0; padding-left: 10px;"> <pre style="white-space: pre-wrap; white-space: \
-moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: \
break-word;">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?</pre> </blockquote>
<p>On December 14th, 2012, 5:22 p.m., <b>Thomas Lübking</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: \
10px;"> <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: \
-pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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.</pre> </blockquote>
<p>On December 14th, 2012, 5:44 p.m., <b>Christoph Feck</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: \
10px;"> <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: \
-pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">> 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.</pre> \
</blockquote>
<p>On December 14th, 2012, 5:58 p.m., <b>Thomas Lübking</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: \
10px;"> <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: \
-pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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.</pre> </blockquote>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: \
-pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">If we'd just turn \
off that "A folder cannot be dropped onto itself" message, a huge annoyance \
would be already gone ;) *scnr*</pre> <br />
<p>- Kai Uwe</p>
<br />
<p>On December 13th, 2012, 9:10 p.m., Thomas Lübking wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" \
style="background-image: \
url('http://git.reviewboard.kde.org/media/rb/images/review_request_box_top_bg.png'); \
background-position: left top; background-repeat: repeat-x; border: 1px black \
solid;"> <tr>
<td>
<div>Review request for Dolphin and Frank Reininghaus.</div>
<div>By Thomas Lübking.</div>
<p style="color: grey;"><i>Updated Dec. 13, 2012, 9:10 p.m.</i></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" \
style="border: 1px solid #b8b5a0"> <tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: \
-moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: \
break-word;">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</pre> </td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: \
1px solid #b8b5a0"> <tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: \
-moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: \
break-word;">clickdragged a folder in both view (w/ and w/o scroll offset)</pre> \
</td> </tr>
</table>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>
<a href="http://bugs.kde.org/show_bug.cgi?id=283646">283646</a>,
<a href="http://bugs.kde.org/show_bug.cgi?id=297414">297414</a>,
<a href="http://bugs.kde.org/show_bug.cgi?id=307747">307747</a>,
<a href="http://bugs.kde.org/show_bug.cgi?id=311483">311483</a>
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>dolphin/src/kitemviews/kitemlistcontroller.h <span style="color: \
grey">(235e4a9)</span></li>
<li>dolphin/src/kitemviews/kitemlistcontroller.cpp <span style="color: \
grey">(697e04f)</span></li>
<li>dolphin/src/views/draganddrophelper.cpp <span style="color: \
grey">(f81d4d0)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/107708/diff/" style="margin-left: \
3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic