Review Request 129601: Left mouse double click fires a "go up"

[Emmanuel Pescosta]

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



Looks good otherwise, thanks for the patch!


src/kitemviews/kitemlistcontroller.h (line 243)
<https://git.reviewboard.kde.org/r/129601/#comment67830>

    Please rename it to `viewDoubleClicked` because we use view instead of no-item in other places too (e.g. `viewContextMenuRequested`)



src/kitemviews/kitemlistcontroller.cpp (lines 804 - 812)
<https://git.reviewboard.kde.org/r/129601/#comment67831>

    Please simplify this a little bit:
    
    if (leftMouseButton) {
        if (validIndex) {
            if (!(m_view->style()->styleHint(QStyle::SH_ItemView_ActivateItemOnSingleClick) || m_singleClickActivationEnforced)) {
    
            }
        } else {
    
        }
    }



src/settings/dolphin_generalsettings.kcfg (line 84)
<https://git.reviewboard.kde.org/r/129601/#comment67832>

    here and everywhere else: DoubleClickGoUp -> DoubleClickViewToGoUp



src/views/dolphinview.cpp (line 153)
<https://git.reviewboard.kde.org/r/129601/#comment67833>

    Personal taste: I would prefer if you put this into a new method instead of a lambda and please add brackets for the if.


- Emmanuel Pescosta


On Dec. 4, 2016, 9:09 a.m., Lorenz Haas wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129601/
> -----------------------------------------------------------
> 
> (Updated Dec. 4, 2016, 9:09 a.m.)
> 
> 
> Review request for Dolphin and KDE Usability.
> 
> 
> Repository: dolphin
> 
> 
> Description
> -------
> 
> If you double click without activating an item, you go up the folder hierarchy.
> 
> BUG: 307505
> 
> 
> Diffs
> -----
> 
>   src/dolphinmainwindow.cpp e5103fd 
>   src/kitemviews/kitemlistcontroller.h b8a93ed 
>   src/kitemviews/kitemlistcontroller.cpp 1c86ff0 
>   src/settings/dolphin_generalsettings.kcfg c724afc 
>   src/settings/navigation/navigationsettingspage.h 5410a4e 
>   src/settings/navigation/navigationsettingspage.cpp e37a35d 
>   src/views/dolphinview.h 0b0d819 
>   src/views/dolphinview.cpp 4105628 
> 
> Diff: https://git.reviewboard.kde.org/r/129601/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Lorenz Haas
> 
>

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