Review Request: Add plasma-themeability to the menu on the presence applet

Dan Vrátil dvratil at redhat.com
Sat Dec 1 16:11:15 UTC 2012


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/107545/#review22870
-----------------------------------------------------------


Nice work! 

This is not your fault, but whoever wrote the original getIcon() code obviously did not care much about coding style, so let's fix it now :)


src/presenceapplet.h
<http://git.reviewboard.kde.org/r/107545/#comment17445>

    Should be
    
    KIcon getIcon(const QString &iconBaseName) const;
    
    Maybe consider calling it getThemedIcon() ?



src/presenceapplet.cpp
<http://git.reviewboard.kde.org/r/107545/#comment17446>

    Put spaces around the operator. Also since you are using the same string below, you could create a single QString above and use it in both places



src/presenceapplet.cpp
<http://git.reviewboard.kde.org/r/107545/#comment17447>

    Space after the comma



src/presenceapplet.cpp
<http://git.reviewboard.kde.org/r/107545/#comment17448>

    Spaces around operator, also you can just do 
    
    return KIcon(svnIcon.pixmap(....));


- Dan Vrátil


On Dec. 1, 2012, 4 p.m., Andromeda Galaxy wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/107545/
> -----------------------------------------------------------
> 
> (Updated Dec. 1, 2012, 4 p.m.)
> 
> 
> Review request for Telepathy.
> 
> 
> Description
> -------
> 
> The Presence applet will show a Plasma-themed icon on the desktop now. However, the menu items for statuses are still all Oxygen, so the user experience is inconsistent -- if the user clicks on an icon for status, they won't get that icon on the toolbar. The attached diff, if applied to the master, will make it so that those menu items are also Plasma-themed.
> 
> 
> Diffs
> -----
> 
>   src/presenceapplet.h 78ccfbd 
>   src/presenceapplet.cpp 291cde4 
> 
> Diff: http://git.reviewboard.kde.org/r/107545/diff/
> 
> 
> Testing
> -------
> 
> 
> Screenshots
> -----------
> 
> The plasma-themed applet
>   http://git.reviewboard.kde.org/r/107545/s/863/
> 
> 
> Thanks,
> 
> Andromeda Galaxy
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-telepathy/attachments/20121201/8a191ddd/attachment.html>


More information about the KDE-Telepathy mailing list