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

[Lorenz Haas]

> On Dec. 4, 2016, 7:09 p.m., Emmanuel Pescosta wrote:
> > src/kitemviews/kitemlistcontroller.h, line 243
> > <https://git.reviewboard.kde.org/r/129601/diff/2/?file=486996#file486996line243>
> >
> >     Please rename it to `viewDoubleClicked` because we use view instead of no-item in other places too (e.g. `viewContextMenuRequested`)

Hm, obviously the "Fixed" button is not the same as the "Done" button in gerrit :) Since it's my first patch for KDE, what is the prefered workflow:

a) I load up a new diff and you press "Fixed" if you accept it
or
b) I say "Fixed" and upload a new diff and you re-open if needed
or 
c) ???

Thanks.


> On Dec. 4, 2016, 7:09 p.m., Emmanuel Pescosta wrote:
> > src/kitemviews/kitemlistcontroller.cpp, lines 804-813
> > <https://git.reviewboard.kde.org/r/129601/diff/2/?file=486997#file486997line804>
> >
> >     Please simplify this a little bit:
> >     
> >     if (leftMouseButton) {
> >         if (validIndex) {
> >             if (!(m_view->style()->styleHint(QStyle::SH_ItemView_ActivateItemOnSingleClick) || m_singleClickActivationEnforced)) {
> >     
> >             }
> >         } else {
> >     
> >         }
> >     }

Done.


> On Dec. 4, 2016, 7:09 p.m., Emmanuel Pescosta wrote:
> > src/settings/dolphin_generalsettings.kcfg, line 84
> > <https://git.reviewboard.kde.org/r/129601/diff/2/?file=486998#file486998line84>
> >
> >     here and everywhere else: DoubleClickGoUp -> DoubleClickViewToGoUp

Done. In addition I also changed the texts (here and elsewhere) to "Double clicking on the view changes to the folder above the current folder" like Don has suggested.

I am not sure what the exact difference is between the text in the kcfg file and navigationsettingspage.cpp? Should it be always the same text?


> On Dec. 4, 2016, 7:09 p.m., Emmanuel Pescosta wrote:
> > src/views/dolphinview.cpp, line 153
> > <https://git.reviewboard.kde.org/r/129601/diff/2/?file=487002#file487002line153>
> >
> >     Personal taste: I would prefer if you put this into a new method instead of a lambda and please add brackets for the if.

Done. I rely on your taste since I am not familiar with the KDE style yet.


- Lorenz


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


On Dec. 4, 2016, 8: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, 8: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/4c2dcf8b/attachment.htm>