[KDE Usability] Review Request 123253: dolphin: Navigate to parent folder selects child folder

[Emmanuel Pescosta]

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



src/dolphinviewcontainer.cpp (line 575)
<https://git.reviewboard.kde.org/r/123253/#comment60730>

    remove or add a proper debug output (qCDebug)



src/dolphinviewcontainer.cpp (line 576)
<https://git.reviewboard.kde.org/r/123253/#comment60731>

    auto -> QUrl and constify



src/dolphinviewcontainer.cpp (line 579)
<https://git.reviewboard.kde.org/r/123253/#comment60732>

    remove spaces after { and before }



src/tests/kfileitemmodeltest.cpp (line 100)
<https://git.reviewboard.kde.org/r/123253/#comment60735>

    please add a new test



src/urlutil.h (line 25)
<https://git.reviewboard.kde.org/r/123253/#comment60733>

    use a namespace instead of a class
    
    but ... maybe we can move firstChildUrl to global.h/.cpp?



src/urlutil.h (lines 29 - 41)
<https://git.reviewboard.kde.org/r/123253/#comment60734>

    Maybe you can add a nicer documentation instead of showing test cases ;)
    
    /**
    Does blablabla ...
    And one short example
    
    @param p1 ...
    
    @result Returns an invalid url when \a currentUrl is not a parent url of ..., the ... otherwise.
    */



src/urlutil.h (lines 49 - 57)
<https://git.reviewboard.kde.org/r/123253/#comment60738>

    path1 = adjustedCurrentUrl.pathWithoutTrailingSlash()
    path2 = adjustedLastUrl.pathWithoutTrailingSlash()
    
    // must hold for parent urls without trailing slashes 
    assert path2.len >= (path1.len + 2)
    
    idx2 = path2.indexOf('/', path1.len + 2)
    len2 = (idx2 < 0) ? path2.len : idx2
    
    path3 = path2.left(len2)
    
    assert path3.numberOfSlashes == (path1.numberOfSlashes + 1)
    
    QUrl res = lastUrl;
    res.setPath(path3)


- Emmanuel Pescosta


On Nov. 19, 2015, 11:31 a.m., Gregor Mi wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/123253/
> -----------------------------------------------------------
> 
> (Updated Nov. 19, 2015, 11:31 a.m.)
> 
> 
> Review request for Dolphin, KDE Usability 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/kfileitemmodeltest.cpp eba32e1e1caa341e1db26399b4027a86614309fc 
>   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/20151119/7e42d739/attachment.htm>
-------------- next part --------------
_______________________________________________
kde-usability mailing list
kde-usability at kde.org
https://mail.kde.org/mailman/listinfo/kde-usability