Review Request: Mouse wheel interaction with breadcrumb-style address bar

[Peter Penz]

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



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

    Please initialize m_wheelSteps with a defined value and move it to the same position as in the header.
    
    Also please add m_subDirNames() at the end (I'm aware that it will be initialized, but it is a recommendation in Effective C++)



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

    Please add a
    } else {
        KUrlButton::wheelEvent(event);
    }



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

    const QString name = ...



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

    Please use
    if ((name != QLatin1String(".")) && (name != QLatin1String(".."))
    
    I know in other parts or the KUrlNavigator code this is missing to, but I'll adjust this.



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

    Can be replaced by
    emit clicked(url, Qt::LeftButton)
    after removing the m_parent member.



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

    Please move the member below m_pendingTextChanges because of the padding.



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

    One goal of the previous refactoring was to get rid of cyclic dependencies from the URL buttons back to the KUrlNavigator -> please remove the member again and replace m_parent->setUrl(...) by emitting the signal clicked(...).
    
    I just recognized that my refactorization was not totally finished yet: The signature of the constructor still contains KUrlNavigator* instead of QWidget*... After you've applied the patch, I'll change this and also rename KUrlButton to KUrlNavigatorButtonBase for consistency.


- Peter


On 2010-04-14 18:51:53, Todd wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/2330/
> -----------------------------------------------------------
> 
> (Updated 2010-04-14 18:51:53)
> 
> 
> Review request for Dolphin and kdelibs.
> 
> 
> Summary
> -------
> 
> This patch allows mouse wheel interaction with the breadcrumb (not editable) address bar used in programs like Dolphin.  Currently this version of the address bar does not have any mouse wheel interaction, so this patch does not interfere with existing functionality.  
> 
> There are two different interactions supported:  
> 
> First, when the vertical mouse wheel (any normal mouse wheel) is used on one of the folders, it moves through the folders in the same level in alphabetical order.  So say you have a folder in your Home directory named "test", with 5 sub-folders, folder1, folder2, folder3, folder4, and folder5.  You are currently in folder3.  If you put your mouse over the folder3 entry in the address bar and rotate your mouse wheel down by one notch, you will switch to folder4.  If you rotate your mouse wheel up by one notch, you will move to folder2.  Move down and up by 2 (or more, in this case) notches moves you to folder5 and folder1, respectively.  While in any of these folders, using your mouse wheel on the "test" folder entry will cycle through the other subfolders in your Home directory (since those folders are at the same level as test).  When you reach the first or last folder in the directory further mouse wheel activity in that direction does nothing.
> 
> The second functionality is provided when doing a horizontal scroll (generally alt+wheel) anywhere on the breadcrumb-style address bar.  In this case, rotating the wheel by one notch in either direction moves you up one directory.  In other words, holding alt and rotating the mouse wheel by one notch is equivalent to hitting the "Up" toolbar button once.  
> 
> This patch does not change the mouse wheel behavior of the traditional text-based (editable) address bar.
> 
> I know this won't make it in before 4.5.
> 
> 
> Diffs
> -----
> 
>   /trunk/KDE/kdelibs/kfile/kurlnavigatorbutton_p.h 1114566 
>   /trunk/KDE/kdelibs/kfile/kurlnavigatorbutton.cpp 1114566 
> 
> Diff: http://reviewboard.kde.org/r/2330/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Todd
> 
>