Review Request 109595: active-welcome plasmoid: use plasmacomponents ToolButton

[Sebastian Kügler]

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


I've noted two small issues in the review, whitespace and high-dpi compatibility.


applets/active-welcome/contents/code/welcome.qml
<http://git.reviewboard.kde.org/r/109595/#comment22050>

    whitespace. Don't leave trailing spaces on empty lines.
    
    Kate (and kwrite and kdevelop) have options to shorten empty lines, please use these. At least KDevelop can be setup so it only removes trailing spaces from lines you've actually changed, so you don't mess up your diff with a lot of whitespace changes that have been done automatically by your editor.



applets/active-welcome/contents/code/welcome.qml
<http://git.reviewboard.kde.org/r/109595/#comment22049>

    Please use theme.iconSizes.desktop here, hardcoded values will cause highdpi issues.


- Sebastian Kügler


On March 19, 2013, 11:08 p.m., Michael Bohlender wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/109595/
> -----------------------------------------------------------
> 
> (Updated March 19, 2013, 11:08 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Description
> -------
> 
> Replace ?mobilecomponents?.IconButton with PlasmaComponents.ToolButton
> 
> I estimated the button size. they might seem visually smaller but the surface you can hit to trigger should be about the same.
> 
> This is my fist patch to Plasma Active.
> I did not subscribe to the plasma list when I first tried to submit this to reviewboard. 
> So sorry for the spam if this comes through twice.
> 
> 
> Diffs
> -----
> 
>   applets/active-welcome/contents/code/welcome.qml cf49d2f 
> 
> Diff: http://git.reviewboard.kde.org/r/109595/diff/
> 
> 
> Testing
> -------
> 
> build + installed with kdesrc-build. viewed with plasmoidviewer
> 
> could not test the video playback as that didn't work before either.
> 
> 
> Thanks,
> 
> Michael Bohlender
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20130320/572bc458/attachment.html>