Review Request 110371: Feature Request 312296 - open selected folders in new tabs

[Frank Reininghaus]

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


Thanks for the patch! I agree that this makes things more consistent, and I guess that people who won't use the feature won't be disturbed too much by the additional menu entries.

However, there are two subtle problems that I had not thought about when I first read the feature request:

1. About adding multiple folders to the "Places": DolphinContextMenu::placeExists(KUrl) sets up a PlacesItemModel to check if the URL is already a "Place". This can cause delays if there are slow devices attached to the computer (there have been some discussions about this issue before the 4.10.0 release because Solid's new UDisks2 backend caused additional delays in some situations, but these issues are mostly resolved now AFAIK). This might not matter much if this function is called just once for a single URL, but if the user selectes hundreds or thousands of folders and then opens the context menu, we might have a problem.

There are ways to work around this problem, of course, but I'm not sure if this is really worth it. In the end, adding multiple selected folders to the "Places" is probably something rather uncommon, so maybe we should not aim to be 100% consistent with the "single folder selected" case here.

2. About "Open in new window(s)": When I first read that report, I thought that opening each folder in a separate new window would be the most consistent solution. I see that you have done it differently, and probably you're right - if there are hundreds of folders selected and we try to run a new Dolphin instance for each of them, we definitely have a problem.

But then I wonder how much the "Open in new window" action is actually worth when multiple folders are selected: it's not really consistent with the "single folder opened in a separate new window" case any more, and I guess it's not really a thing that users will do frequently anyway - "Open all folders in tabs in a new window" and "Open all folders in tabs in the existing window" are not that much different from each other.



Therefore, I suggest to only add the "Open in new tabs" action if multiple folders are selected. Sorry that I had suggested something different in my comment in the wish report. Some of the potential problems that the "let's try to be as consistent as possible with the single folder case" approach has only become obvious when you think about this issue for some time.



- Frank Reininghaus


On May 9, 2013, 1:02 p.m., Emmanuel Pescosta wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/110371/
> -----------------------------------------------------------
> 
> (Updated May 9, 2013, 1:02 p.m.)
> 
> 
> Review request for Dolphin and Frank Reininghaus.
> 
> 
> Description
> -------
> 
> Make the item context menu entries: "Open in new Window", "Open in new Tab" and "Add to places" work for more than one selected folder.
> 
> Modified all involved functions to handle multiple selected items.
> 
> Check if the folders are already in the Places Model, before we add them to the Places Model in DolphinContextMenu::openItemContextMenu().
> 
> Also modified the KRun url from "dolphin %u" to "dolphin %U" -> All items are opened in the same new Dolphin Window.
> 
> 
> This addresses bug 312296.
>     http://bugs.kde.org/show_bug.cgi?id=312296
> 
> 
> Diffs
> -----
> 
>   dolphin/src/dolphincontextmenu.cpp 89a169f 
>   dolphin/src/dolphinmainwindow.cpp 347489d 
> 
> Diff: http://git.reviewboard.kde.org/r/110371/diff/
> 
> 
> Testing
> -------
> 
> Works for me!
> 
> * Select multiple files and folder:
>   None of these three actions are visible.
> 
> * Select multiple files:
>   None of these three actions are visible.
> 
> * Select multiple folders (All folders are already in the Places Panel):
>   Only the actions "Open in new Window" and "Open in new Tab" are visible.
> 
> * Select multiple folders (Not all folders are in the Places Panel):
>   All three actions are visible.
> 
> 
> Thanks,
> 
> Emmanuel Pescosta
> 
>

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