Review Request 123253: dolphin: Navigate to parent folder selects child folder

[Frank Reininghaus]

> On Jan. 29, 2016, 3:24 nachm., Frank Reininghaus wrote:
> > What I still do not understand is why you want to make this change in Dolphin, and not in KUrlNavigator, as I have suggested in another comment here some time ago.
> > 
> > Your patch will make the "go up" behavior inconsistent in Dolphin and the file dialog, which is rather bad for the user experience IMHO. If you change KUrlNavigator, then everything will be consistent, and code maintenance will be easier. Moreover, the risk of subtle bugs (by the two layers of history handling which your patch introduces) will be lower.
> 
> Gregor Mi wrote:
>     Hi, in some earlier comments I noted that I had difficulties implementing this in KUrlNavigator (can't remember why excactly, but I could look it up later).
>     
>     I share your concern with the inconsistency issue between dolphin and file dialog. Right now, I see this as a lower priority because...
>     
>     1) it was not sure if the feature was accepted at all and I wanted to present a working implementation.
>     2) I think most non-power users won't notice the feature at all.
>     3) In my personal experience "heavy-duty" navigating takes place in dolphin but not in the file dialog. So, the lack of the feature there would not be that bad. 
>     
>     That said, I think it should indeed be moved to KUrlNavigator or some other shared location to make the behaviour consistent, if time proves that the new behaviour is good to keep.

> Hi, in some earlier comments I noted that I had difficulties implementing this in KUrlNavigator (can't remember why excactly, but I could look it up later).

I looked it up. This was because there are different ways to change the behavior concerning "Navigating to parent folder":

1. Assume that /home/user is shown in the view, then the user clicks /home/user/subfolder to navigate to "subfolder", then clicks "up" to go to the home folder again. Currently, the view will then show /home/user scrolled to the top, but it seems that some users expect that it should be scrolled to "subfolder", and "subfolder" should be the active item. Essentially, this could be achieved by interpreting "go up" as "go back" if the "up" URL is that same URL that would be reached by the "back" action. This would be easy to implement in KUrlNavigator.

2. What you want is something different: If /home/user is shown in the view, and then "/" is entered in the location bar, then you want that "home" is selected. This is difficult to implement in KUrlNavigator.

Now the question is if solution 2 is really so much better than solution 1 that inconsistencies and more complex code (the two different kinds of history handling in KUrlNavigator and in DolphinViewConainer's m_lastUrl still look wrong to me) are justified. I think I would actually find behavior 2 confusing, to be honest. For example, if "/" is shown in the view, /usr is the current item, and I click "Home" in the Places, and then click "Back": Currently, the original state of the "/" view would be restored, i.e., /usr would still be the current item, and I think that this is what would be expected from the "Back" action: go back, and show the view just like it was. This is what the history is for IMHO. With your patch, however, /home would be the current item in this situation. This looks
  wrong to me.

> 1) it was not sure if the feature was accepted at all and I wanted to present a working implementation.
> 2) I think most non-power users won't notice the feature at all.
> 3) In my personal experience "heavy-duty" navigating takes place in dolphin but not in the file dialog. So, the lack of the feature there would not be that bad.

I don't see how any of this justifies inconsistencies between the behavior of Dolphin and the file dialog.

> That said, I think it should indeed be moved to KUrlNavigator or some other shared location to make the behaviour consistent, if time proves that the new behaviour is good to keep.

This might not be possible at all. Once Dolphin is released with a modified behavior that adds some custom history handling on top of the one in KUrlNavigator, then any future change in KUrlNavigator's behavior might interact badly with the modifications in Dolphin. Note that both have different release schedules, so it would not be possible to fix this situation by reverting the modifications in Dolphin.


- Frank


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/123253/#review91783
-----------------------------------------------------------


On Jan. 24, 2016, 4:14 nachm., Gregor Mi wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/123253/
> -----------------------------------------------------------
> 
> (Updated Jan. 24, 2016, 4:14 nachm.)
> 
> 
> Review request for Dolphin and Emmanuel Pescosta.
> 
> 
> Bugs: 335616
>     https://bugs.kde.org/show_bug.cgi?id=335616
> 
> 
> Repository: dolphin
> 
> 
> Description
> -------
> 
> This is a first working implementation of the feature suggestion filed in the ticket https://bugs.kde.org/show_bug.cgi?id=335616: "Dolphin: Navigate to parent folder selects child folder".
> 
> In short, this is what is does: Whenever the dolphin view is initialized to show the contents of a new URL (e.g. "/home/x/test") it will be checked if the new URL is a parent of the previous displayed URL (e.g. "/home/x/test/documents/aaa"). If the check is successful, then the common child (in this example: "/home/x/test/documents/") folder item will be selected and scrolled into view.
> 
> 
> Diffs
> -----
> 
>   src/dolphinviewcontainer.h 62f91100e9e5d457edd6f4d927c87610335834d7 
>   src/dolphinviewcontainer.cpp 8fea3ba9d0bb8389d89724b9f0cd74605c0286fe 
>   src/tests/CMakeLists.txt 22a8b48491fa7ac88ce1b29aecb20192837dd7ea 
>   src/tests/urlutiltest.cpp PRE-CREATION 
>   src/urlutil.h PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/123253/diff/
> 
> 
> Testing
> -------
> 
> - unit test passes
> - Played around with dolphin: enter URL manually, navigate via click in the item view, navigate via click in kurlnavigator, navigate with Alt+Left, Alt+Right, Alt+up, Backspace
> 
> 
> Thanks,
> 
> Gregor Mi
> 
>

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