--===============3995898388818621611== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable > On Dec. 14, 2012, 3:56 p.m., Frank Reininghaus wrote: > > First of all, thanks for the patch! I see now that dragging items accid= entally 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 D= ragAndDropHelper depend on KItemListController and adding the hand-made pre= ss/release events. I have two ideas at the moment: > > = > > 1. Make KItemListController finally respect http://qt-project.org/doc/q= t-4.8/qapplication.html#startDragTime-prop, and only start a drag in KItemL= istController::mouseMoveEvent() if the time since the mouse press event is = larger than that, or > > = > > 2. Check in KItemListController::mouseMoveEvent() if the mouse cursor i= s 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 id= ea 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 poi= nt of view. But maybe there is a good reason why you chose to do it in a di= fferent way? > = > Thomas L=C3=BCbking 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 f= or the first half second nothing happens) - esp. since apparently no Q/KIte= mView seems to care much about it. > = > If you'd advice ppl. to shorten the timeout to sth. "reasonable" (bel= ow 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 (=3D startDragDistance), this is the ex= pected (and correct) behaviour. Try window dragging by click-move on the ti= tle bar. > = > Thomas L=C3=BCbking wrote: > Ok, thanks (never waited so log ;-) > Just that the expected "autodrag w/o move" won't help on the particul= ar issue (since the drag distance is respected and effectively the problem) > = > @Frank: we could simply keep the dragTime in the DragAndDropHelper in= stead to avoid deps. on the controller. > = > Kai Uwe Broulik wrote: > If we'd just turn off that "A folder cannot be dropped onto itself" m= essage, a huge annoyance would be already gone ;) *scnr* > = > Frank Reininghaus wrote: > Thanks Thomas for the new patch and everyone else for the comments, i= n particular Christoph - I somehow thought the idea was to start a drag whe= n "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 it= self" 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 th= is is a lot more complex than it needs to be. Why not just put the timer in= to KItemListController, start it when the mouse button is pressed, and in m= ouseMoveEvent, if the drag distance is too small, check if the time since t= he press is larger than startDragTime and do not start a drag if that is no= t the case. > = > This should mostly yield the same result and require a lot less code = and no dependencies between KItemListController and DragAndDropHelper, or a= m I overlooking anything? > = > Thomas L=C3=BCbking wrote: > This will effectively increase the QApplication::startDragDistance(),= because for the first 500ms -assuming default value in place- the drag dis= tance is increased to not "too small" and 500ms is much time in this contex= t. > = > You can try the outcome by increasing it legally in "kcmshell4 mouse"= - it's capped @ 20px, though, while a quick judder (the typical movement i= s down and up) can be much more (esp. depending on screen resolution and m= ouse acceleration) > = > I'm willing add an update, but notice that the approach omits the bes= t variable for the heuristic (button release, ie. drag time) and trap us be= tween false positives (dialog/notification) and laggy DnD behavior (even wi= th 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", b= ut 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 distr= acting 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 requ= ires a lot less code. Moreover, I'm worried that the hand-made press and re= lease events might lead to strange bugs in the future. They are delivered i= nside the nested event loop of the drag, and I've really had a lot of unple= asant experiences with nested event loops in the past. > = > I see that replacing the check "distance > startDragDistance" by "dis= tance > startDragDistance || time > startDragTime" would not resolve the "a= ccidental 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) det= ermine 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 tha= t earlier, but nobody commented on that idea. Am I overlooking anything whi= ch is obviously wrong with that idea? > > = > Thomas L=C3=BCbking wrote: > > not starting the drag if we think that the user does not want to dr= ag 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 miscl= ick 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 pubi= shment 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 di= rty 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 tr= eatment of user failure. > Usually, if a DnD cannot take place, the action silently quits (previ= ously 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 (the= re's a setting for this and dolphin does not differ from any other client i= n that regard) but on failure treatment. > = > This could also be a timeout on just showing the dialog - i just thou= ght 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'. T= herefore, I don't see why we would get "dolpin does not respect dnd startdi= stance" 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 li= ke after a click, and will trigger the item. > = > This means: a drag is only started when the cursor moves out of the b= ounds of the item where the mouse was pressed AND if the distance between p= ress and release is larger than the 'start drag distance', whatever the use= r chose for it. = > = > Before we continue the discussion, could please anyone tell me what i= s wrong with this approach? > = > About the 'folder cannot be dropped on itself' message: I didn't inve= nt it, and from my point of view it's fine to replace it with something els= e, 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 m= oment). > = > Thomas L=C3=BCbking wrote: > Sorry, i was abroad (need to move reviewboard to gmail) > = > > 1. When the mouse moves, we check if the mouse cursor is still abov= e the item where the button was pressed. If that is the case, we don't star= t a drag > = > What effectively increases the startDragDistance. > = > The default value here is 4px while the minimum icon size is 16px, ma= ny users will use up to 64px (or even more, eventually plus 2-3 lines of te= xt) what means a startDragDistance of 8px - 32px (if you click the center a= nd move straight out). > = > I do not discuss in terms of wrong or right (outside math ;-) but the= re's a notable difference in the behavior if you enforce to leave an item t= o start the DnD which grows with the icon size. > Assumption that there'll be a bug report about it is just "bugzilla e= xperience gut feeling" - i've no proof for that =3D) > = > Frank Reininghaus wrote: > Thanks for the clarification and sorry for the late reply (real life = kept me busy around Christmas)! > = > You are right - I had only considered the fact that users probably ne= ver want to start a drag and then drop on the same item. But if the "drag p= ixmap" is only shown after the mouse cursor has left the item, then this do= es feel different, and there is indeed the possibility that some people mig= ht consider this annoying. > = > I think the best way to improve the user experience might be to show = a "drop forbidden" cursor while hovering the item where the drag started, r= ather than accepting the drop and then showing the annoying error message. > = > I see that accepting the drop and then "canceling" the drag after it = happened might also help to reduce the effects of the annoying message, but= I think that the "fake" events are likely to cause trouble at some point i= n the future :-( Nevermind the lag. You could still show the dialog after a longer drag (to hint that this is i= llegal by intent) but silently fail on a short term judder (assuming that p= pl. won't be able to coordinate the perception of the started drag and then= drop the item within less then 500ms while nobody will hold the button tha= t long for a click) That includes no sythetic events at all. - 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=C3=BCbking 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 mo= ved 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 w= arning 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 applica= tion 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 to= o 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. Eithe= r because they intend to link it there (link recursion can be dangerous, bu= t is a legal action) or for "ummm... i didn't want to copy that folder. err= r... how do i stop this ... ok, let's just put it back from where it came a= nd 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 acco= unt that by dropping the timeout > = > = > This addresses bugs 283646, 297414, 307747, and 311483. > http://bugs.kde.org/show_bug.cgi?id=3D283646 > http://bugs.kde.org/show_bug.cgi?id=3D297414 > http://bugs.kde.org/show_bug.cgi?id=3D307747 > http://bugs.kde.org/show_bug.cgi?id=3D311483 > = > = > 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=C3=BCbking > = > --===============3995898388818621611== Content-Type: text/html; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable
This is an automatically generated e-mail. To reply, visit: http://git.revie= wboard.kde.org/r/107708/ |
On December 14th, 2012, 3:56 p.m., Frank Re= ininghaus 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 D= ragAndDropHelper depend on KItemListController and adding the hand-made pre= ss/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 KItemListC= ontroller::mouseMoveEvent() if the time since the mouse press event is larg= er than that, or 2. Check in KItemListController::mouseMoveEvent() if the mouse cursor is st= ill above the item where the press happened, and do not start a drag if tha= t is the case. Both approaches would prevent that the drag is started, whereas your idea w= as to sort of cancel the drag after it happened. Not starting the drag in t= he first place might be a bit cleaner and require less code from my point o= f view. But maybe there is a good reason why you chose to do it in a differ= ent way?On December 14th, 2012, 5:22 p.m., Thomas L=C3=BCbking wrote:
Supportin= g 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 ha= lf 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&qu= ot; (below 150ms), they'll again run into the accidental drags. I'll update the patch later on to simply respect the timeout so that yo= u can try for yourself.On December 14th, 2012, 5:44 p.m., Christoph Feck wrote:
> you = press, hold, drag and for the first half second nothing happens Unless you drag it far enough (=3D startDragDistance), this is the expected= (and correct) behaviour. Try window dragging by click-move on the title ba= r.On December 14th, 2012, 5:58 p.m., Thomas L=C3=BCbking wrote:
Ok, thank= s (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.On December 14th, 2012, 6:52 p.m., Kai Uwe Broulik wrote:
If we'= ;d just turn off that "A folder cannot be dropped onto itself" me= ssage, a huge annoyance would be already gone ;) *scnr*On December 16th, 2012, 7:12 p.m., Frank Reininghaus wrote:
Thanks Th= omas for the new patch and everyone else for the comments, in particular Ch= ristoph - I somehow thought the idea was to start a drag when "distanc= e > startDragDistance" && "time since press > start= DragTime", but with && replaced by ||, it makes much more sens= e 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 mou= seMoveEvent, 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 ov= erlooking anything?On December 16th, 2012, 8:19 p.m., Thomas L=C3=BCbking wrote:
This will= effectively increase the QApplication::startDragDistance(), because for th= e first 500ms -assuming default value in place- the drag distance is increa= sed to not "too small" and 500ms is much time in this context. You can try the outcome by increasing it legally in "kcmshell4 mouse&q= uot; - it's capped @ 20px, though, while a quick judder (the typical mo= vement is down and up) can be much more (esp. depending on screen resoluti= on 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 betw= een 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 (fr= om the controller) store the msecsSinceReference (kinda "current time&= quot;, but monotic) as dynamic property "DolphinDragStartTime" on= the QApplication object and fetch and compare it from there DnDHelper.On December 18th, 2012, 7:29 a.m., Frank Reininghaus wrote:
Well, I= 39;m not a usability expert, but IMHO, not starting the drag if we think th= at 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 rele= ase events might lead to strange bugs in the future. They are delivered ins= ide the nested event loop of the drag, and I've really had a lot of unp= leasant experiences with nested event loops in the past. I see that replacing the check "distance > startDragDistance" = by "distance > startDragDistance || time > startDragTime" w= ould 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 e= ither). What about the following idea: I guess that users never drag an item to dro= p it on itself. We could (in KItemListController::mouseMoveEvent) determine= the 'index' which belongs to the position of the event and just no= t start the drag if this index is equal to m_pressedIndex. I've suggest= ed that earlier, but nobody commented on that idea. Am I overlooking anythi= ng which is obviously wrong with that idea?On December 18th, 2012, 2:42 p.m., Thomas L=C3=BCbking 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 increa= se (by tests in the former and last comments) what's (given the misclic= k isn't a dolphin only phenomenon) equal to telling the user to do so i= n 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 d= oen not respect dnd startdistance" bugs. The startdistance actually exists for this purpose (smoothing away dirty cl= icks) and if usually sufficient, it's not reasonable to increase the se= tting, nor hardcode a higher value (just think of 128px icons...) What is special about dolphin and source of the bug reports is its treatmen= t of user failure. Usually, if a DnD cannot take place, the action silently quits (previously = hinted by the "forbidden" cursor) but dolphin currently "ann= oys" the user with a (ui re-layouting) message (in this case false pos= itive 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 th= at since we pretty much (now!) know what the user intended, we could just d= o that for him.On December 18th, 2012, 5:32 p.m., Frank Reininghaus wrote:
Well, eit= her I don't understand you, or you don't understand me, but the res= ult 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 it= em 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 t= han the 'start drag distance'). 2. Because no drag was started, mouseReleaseEvent will behave just like aft= er 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 a= nd release is larger than the 'start drag distance', whatever the u= ser chose for it. = Before we continue the discussion, could please anyone tell me what is wron= g 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 som= ething 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 d= on't know at the moment).On December 21st, 2012, 9:23 p.m., Thomas L=C3=BCbking wrote:
Sorry, i = was abroad (need to move reviewboard to gmail) > 1. When the mouse moves, we check if the mouse cursor is still above t= he item where the button was pressed. If that is the case, we don't sta= rt a drag What effectively increases the startDragDistance. The default value here is 4px while the minimum icon size is 16px, many use= rs will use up to 64px (or even more, eventually plus 2-3 lines of text) wh= at means a startDragDistance of 8px - 32px (if you click the center and mov= e 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 "bugzill= a experience gut feeling" - i've no proof for that =3D)On December 27th, 2012, 9:04 p.m., Frank Reininghaus wrote:
Thanks fo= r the clarification and sorry for the late reply (real life kept me busy ar= ound Christmas)! You are right - I had only considered the fact that users probably never wa= nt to start a drag and then drop on the same item. But if the "drag pi= xmap" is only shown after the mouse cursor has left the item, then thi= s does feel different, and there is indeed the possibility that some people= might consider this annoying. I think the best way to improve the user experience might be to show a &quo= t;drop forbidden" cursor while hovering the item where the drag starte= d, rather than accepting the drop and then showing the annoying error messa= ge. I see that accepting the drop and then "canceling" the drag after= it happened might also help to reduce the effects of the annoying message,= but I think that the "fake" events are likely to cause trouble a= t some point in the future :-(
Nevermind t= he lag. You could still show the dialog after a longer drag (to hint that this is i= llegal by intent) but silently fail on a short term judder (assuming that p= pl. won't be able to coordinate the perception of the started drag and = then drop the item within less then 500ms while nobody will hold the button= that long for a click) That includes no sythetic events at all.
- Thomas
On December 14th, 2012, 9:25 p.m., Thomas L=C3=BCbking wrote:
Review request for Dolphin and Frank Reininghaus.
By Thomas L=C3=BCbking.
Updated Dec. 14, 2012, 9:25 p.m. Descripti= on
Testing <= /h1>
Diffs=
|