[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-core-devel
Subject:    Re: Review Request: Mouse wheel interaction with breadcrumb-style
From:       "Peter Penz" <peter.penz () gmx ! at>
Date:       2010-04-14 20:29:23
Message-ID: 20100414202923.2971.29053 () localhost
[Download RAW message or body]


-----------------------------------------------------------
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
> 
> 


[prev in list] [next in list] [prev in thread] [next in thread]