Review Request: When breadcrumb bar has more than 100 items show a sub-menu that lists the additional items instead of "..."

[Peter Penz]

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


Thanks Todd for the patch! I'm surprised that the patch got so simple and am very pleased with the result. I've added some suggestions for the code, please commit after fixing them (and if you still don't have an SVN account: I think it is really time that you take this patch as trigger to get one ;-))


/trunk/KDE/kdelibs/kfile/kurlnavigatorbutton.cpp
<http://reviewboard.kde.org/r/3796/#comment4720>

    I'd suggest to rename 'dirCutoff' to 'maxDirIndex'



/trunk/KDE/kdelibs/kfile/kurlnavigatorbutton.cpp
<http://reviewboard.kde.org/r/3796/#comment4721>

    nitpick: missing space after comma :-)



/trunk/KDE/kdelibs/kfile/kurlnavigatorbutton.cpp
<http://reviewboard.kde.org/r/3796/#comment4718>

    The usage of QPointer is useless here, just use 'KUrlNavigatorMenu* subDirsMenu = ...' instead.



/trunk/KDE/kdelibs/kfile/kurlnavigatorbutton.cpp
<http://reviewboard.kde.org/r/3796/#comment4719>

    Please use 'i18nc("@action:inmenu", "More")'



/trunk/KDE/kdelibs/kfile/kurlnavigatorbutton_p.h
<http://reviewboard.kde.org/r/3796/#comment4717>

    I'd suggest to change the signature of the method like this:
    
    void initMenu(KUrlNavigatorMenu* menu, int startIndex);
    
    (no default parameter, no 'const int', renamed from fillDirsMenu() to initMenu(), as more things are done than just filling the menu)
    
    


- Peter


On 2010-04-24 01:39:00, Todd wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/3796/
> -----------------------------------------------------------
> 
> (Updated 2010-04-24 01:39:00)
> 
> 
> Review request for kdelibs.
> 
> 
> Summary
> -------
> 
> Currently, when you have more than 100 items in the dolphin breadcrumb bar it only shows the first 100 items then shows "..." at the end, with no way to access the additional items.  This patch changes the behavior so that instead it shows a sub-menu labeled "More" which shows more items.  If there are more than 100 items in the sub-menu it recursively displays additional sub-menus until it runs out of items.  The sub-menus are standard sub-menus that appear in hovering.
> 
> 
> Diffs
> -----
> 
>   /trunk/KDE/kdelibs/kfile/kurlnavigatorbutton.cpp 1117311 
>   /trunk/KDE/kdelibs/kfile/kurlnavigatorbutton_p.h 1117311 
> 
> Diff: http://reviewboard.kde.org/r/3796/diff
> 
> 
> Testing
> -------
> 
> Opening folders in the menu, dragging and dropping.
> 
> 
> Thanks,
> 
> Todd
> 
>