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

[Frank Reininghaus]

> On May 10, 2013, 9:33 a.m., Frank Reininghaus wrote:
> > 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.
> > 
> >
> 
> Emmanuel Pescosta wrote:
>     Thanks for your feedback :)
>     
>     > 1.
>     Yes I agree with you, just followed your comment from the bug report. I will remove it.
>     
>     > 2. it's not really consistent with the "single folder opened in a separate new window" case any more
>     Hmm what do you mean? - When you have one folder selected and you trigger the "Open in new Window" action, a new Dolphin instance will be started and will load this url. If multiple folders are selected and you trigger the "Open in new Window" action, a new Dolphin instance will be started but it will load all the urls in new tabs. (Same behavior as video files + VLC, audio files + Amarok, images + Gwenview, ... - only one instance for multiple urls) 
>     
>     I see no inconsistent behavior here. ;)
>     
>     So what do you think?
>

>From my point of view, it would be more consistent if the action would be "Open in new windows" (plural) for the case that multiple folders are selected, and each folder would be opened in its own window. But this does obviously not make much sense if many folders are selected and we try to open hundreds of Dolphin instances.

One could argue that opening multiple folders in tabs in a single new window is also sort-of consistent with the "single folder" case, but then I wonder: what is the use case for this? The actions "Open in new tabs" and "Open in new tabs in a new window" do almost the same thing then, and IMHO, it might be confusing to offer both options if they differ only slightly and it's not even clear that both are needed.


- Frank


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


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/20130512/559b61d1/attachment.htm>