Review Request 126300: Plasma Search KCM: display the runner descriptions

Kai Uwe Broulik kde at privat.broulik.de
Thu Jan 7 09:47:21 UTC 2016


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

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20160107/91b7129c/attachment.html>


More information about the Plasma-devel mailing list