Review Request: Fixes Bugs 242007, 293200 and 305783

Sat Sep 1 11:52:39 BST 2012

> On Aug. 31, 2012, 3:09 p.m., Frank Reininghaus wrote:
> > Thanks for looking into these issues and for the patch! And thanks for noticing the Folders Panel expansion problem in the first place.
> > 
> > I agree that the Folders Panel issues should be fixed, but I'm not so sure about opening folders in the view which have been hovered while dragging something to the Places Panel. This would be a behaviour change which might not be suitable for a stable branch. And I'm not really sure if this would be very useful at all, I have to check how other file managers do it.
> > 
> > About updating the auto activation in the Folders Panel when the settings are changed: I agree that this would make Dolphin's behaviour more consistent, and first I thought that this is a very good idea. But then I tried an older Dolphin version (with the old, Qt itemview-based Folders Panel implementation) and noticed that folders were always expanded in that panel even there, no matter what the 'open folders during drag operations' setting is. It's been like that in every Dolphin version so far, which makes me wonder if some people actually rely on that behaviour. Moreover, we never had any complaints about this, so I think it would be safer to always expand folders in the Folders Panel when hovering them with something that has been dragged. This would also mean that we don't need the additions to DolphinMainWindow, which is a very complex class already.
> > 
> > What do you think?
> 
> Emmanuel Pescosta wrote:
>     > I agree that the Folders Panel issues should be fixed, but I'm not so sure about opening folders in the view which have been hovered while dragging something to the Places Panel. This would be a 
>     > behaviour change which might not be suitable for a stable branch. And I'm not really sure if this would be very useful at all, I have to check how other file managers do it.
>     
>     What if we increase the timeout to 1000 ms or so? 
>     In my mind (!) this is a big usability improvement and if I activate "Open folders during drag operations" I also want to use this great feature in the Places Panel, where I have bookmarked my important folders (e.g. Pictures, Downloads, Developing, School, ...) for FAST access. Also drag and drop files to a device (Especially with automount - Bug 176277 :) ) or webdav dir works much better and faster if we do it in this way. Please try it. -> So I'm sure this would be a very useful feature for all Dolphin users :) (What if we add a new option to enable it in settings?)
>     
>     > I agree that this would make Dolphin's behaviour more consistent
>     
>     Yes :) I think a consistent behaviour is very important (at least as important as a consistent UI)
>     
>     > This would also mean that we don't need the additions to DolphinMainWindow
>     
>     Is there a better way to implement this? If yes, I will fix it :)

First of all, you can commit the fix for https://bugs.kde.org/show_bug.cgi?id=305783#c4, of course, this is unrelated to the rest of the discussion. I see that you use m_view->isUnderMouse() to determine if the mouse is still hovering the widget. An alternative (more consistent with what happens in KItemListController::dragMoveEvent()) might be to stop the timer in KItemListController::dragLeaveEvent(), but I don't have a strong opinion about this, just choose the approach you prefer. Note that 4.9.1 has been tagged already, so you have to put 4.9.2 in the FIXED-IN field.

Concerning the automatic expansion in the Folders Panel: What I'm concerned about if we make this consistent with the "Open folders during drag operations" setting is the following.

a) We never had any complaints about this inconsistency AFAIK. Note that bug 293200 is about something else. The bug description is very difficult to understand, look at Peter's comment 3 for a summary of what we consider to be the buggy behaviour.

b) Some people who have never enabled the "Open..." setting might actually rely on the automatic expansion in the Folders Panel and might consider any behaviour change as a bug.

About the Places Panel issue: I think I'm mostly convinced now :-) However, I think this should only go into master because I consider this more a feature than a bug fix, and this might require some fine-tuning later on (like automatic mounting of devices which are hovered during the drag operation).

The way you implemented the refreshing of the panels when the settings change is good, but I'd prefer to only add new member variables to DolphinMainWindow if there is no other solution. We could do it using a new signal settingsChanged() in DolphinMainWindow instead: connect that signal to a slot in the Places Panel in DolphinMainWindow::setupDockWidgets(), and in DolphinMainWindow::editSettings(), connect the dialog's settingsChanged() signal to the DolphinMainWindow's settingsChanged() signal. I haven't tried that yet, but I think it should work, and would not increase the complexity of DolphinMainWindow much.

- Frank

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

On Aug. 30, 2012, 5:55 a.m., Emmanuel Pescosta wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/106271/
> -----------------------------------------------------------
> 
> (Updated Aug. 30, 2012, 5:55 a.m.)
> 
> 
> Review request for Dolphin and Frank Reininghaus.
> 
> 
> Description
> -------
> 
> Fixes Bug 293200 - Drag&drop files in dolphin doesnt preserve origin
> 
> Fixes Bug 242007 - "Open Folder during Drag operation" cannot go into different partition using "Places Pane
> 
> Fixes Bug 305783 - dragging a file over a directory
>      does not expand the dir => Bug discovered: When you drag a
>      item onto a folder-view-item and then move it away
>      instantly before the autoactivation event is triggered
>      (After 750ms), the folder will be opened anyway.
> 
> 
> This addresses bugs 242007, 293200 and 305783.
>     http://bugs.kde.org/show_bug.cgi?id=242007
>     http://bugs.kde.org/show_bug.cgi?id=293200
>     http://bugs.kde.org/show_bug.cgi?id=305783
> 
> 
> Diffs
> -----
> 
>   dolphin/src/dolphinmainwindow.h ab79fb0 
>   dolphin/src/dolphinmainwindow.cpp d83c9de 
>   dolphin/src/kitemviews/kitemlistcontroller.cpp 88f5d9f 
>   dolphin/src/panels/folders/folderspanel.h 14d8e87 
>   dolphin/src/panels/folders/folderspanel.cpp 0760200 
>   dolphin/src/panels/panel.h 064e1f3 
>   dolphin/src/panels/panel.cpp c2681ec 
>   dolphin/src/panels/places/placespanel.h 8a84e00 
>   dolphin/src/panels/places/placespanel.cpp d445088 
> 
> Diff: http://git.reviewboard.kde.org/r/106271/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Emmanuel Pescosta
> 
>

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