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


+1 for the idea

A bunch of coding nitpicks below.


kcms/runners/kcm.h (line 41)
<https://git.reviewboard.kde.org/r/126300/#comment62033>

    nullptr



kcms/runners/kcm.h (line 42)
<https://git.reviewboard.kde.org/r/126300/#comment62032>

    virtual ~SearchConfigItemDelegate() = default;



kcms/runners/kcm.h (line 45)
<https://git.reviewboard.kde.org/r/126300/#comment62034>

    use "override" instead of virtual for overriden methods



kcms/runners/kcm.cpp (line 40)
<https://git.reviewboard.kde.org/r/126300/#comment62035>

    Please sort includes alphabetically (okey, they aren't, so don't change the 
ones that are already there but at least place QApplication at the top :)



kcms/runners/kcm.cpp (line 61)
<https://git.reviewboard.kde.org/r/126300/#comment62036>

    opt.text.clear(); ?



kcms/runners/kcm.cpp (line 72)
<https://git.reviewboard.kde.org/r/126300/#comment62037>

    spaces between operators:
    2 * m_margin



kcms/runners/kcm.cpp (line 83)
<https://git.reviewboard.kde.org/r/126300/#comment62038>

    Spaces between operators: Qt::AlignLeft | Qt::AlignTop


- Kai Uwe Broulik


On Jan. 7, 2016, 8:45 vorm., Jonathan Marten wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126300/
> -----------------------------------------------------------
> 
> (Updated Jan. 7, 2016, 8:45 vorm.)
> 
> 
> Review request for Plasma and KDE Usability.
> 
> 
> Repository: plasma-desktop
> 
> 
> Description
> -------
> 
> In Plasma 5 this KCM only lists the names of the available runners, with no 
> explanation of what they do.  In its previous incarnation in KDE4 (the 
> dropdown list from the runner config button), the descriptions were 
> displayed.  This change restores them.
> 
> 
> Diffs
> -----
> 
>   kcms/runners/kcm.h f1239ee454cf40c3721743d5c771b4686d449d21 
>   kcms/runners/kcm.cpp 4af82de9c385725a23cc09a074c5803f11a7945f 
> 
> Diff: https://git.reviewboard.kde.org/r/126300/diff/
> 
> 
> Testing
> -------
> 
> Built plasma-desktop with this change, checked appearance of KCM in Breeze, 
> Oxygen and older styles.
> 
> 
> File Attachments
> ----------------
> 
> Screenshot before
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2015/12/10/caf0a4d7-ab8d-410a-9409-ae6935d24929__plasmasearch-before-r126300.png
> Screenshot after
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2015/12/10/0b695a2b-4307-4a70-9a9b-4c3bb80f7955__plasmasearch-after-r126300.png
> 
> 
> Thanks,
> 
> Jonathan Marten
> 
>

_______________________________________________
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Reply via email to