Review Request 112888: Roughly port icon applet to plasma2

[Sebastian Kügler]

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



plasma/generic/applets/icon/plugin/icon_p.h
<http://git.reviewboard.kde.org/r/112888/#comment29884>

    the getters should all be const



plasma/generic/applets/icon/plugin/icon_p.h
<http://git.reviewboard.kde.org/r/112888/#comment29891>

    arguments should begin with lowercase letters, url, name, icon, genericName (or if you want to make the distinction between the arg and the method name more clear, use newUrlm newName, etc., or something telling.



plasma/generic/applets/icon/plugin/icon_p.cpp
<http://git.reviewboard.kde.org/r/112888/#comment29890>

    this is only used in the !fi.isDir() branch, move it there for more narrow scoping.



plasma/generic/applets/icon/plugin/icon_p.cpp
<http://git.reviewboard.kde.org/r/112888/#comment29885>

    I'd prefer to move this to the other signal emissions, just to make sure we're done with everything at the same time. Semantically, we're only done setting the url once all internal data is updated.



plasma/generic/applets/icon/plugin/icon_p.cpp
<http://git.reviewboard.kde.org/r/112888/#comment29889>

    space after if



plasma/generic/applets/icon/plugin/icon_p.cpp
<http://git.reviewboard.kde.org/r/112888/#comment29887>

    const



plasma/generic/applets/icon/plugin/icon_p.cpp
<http://git.reviewboard.kde.org/r/112888/#comment29888>

    const



plasma/generic/applets/icon/plugin/icon_p.cpp
<http://git.reviewboard.kde.org/r/112888/#comment29886>

    space after if


- Sebastian Kügler


On Sept. 23, 2013, 2:56 p.m., Bhushan Shah wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/112888/
> -----------------------------------------------------------
> 
> (Updated Sept. 23, 2013, 2:56 p.m.)
> 
> 
> Review request for Plasma, Marco Martin and Sebastian Kügler.
> 
> 
> Description
> -------
> 
> Port Icon applet to plasma2
> 
> Some issues..
> 
> -> Can be resized to 1x1.
> -> Still doesn't work for folder, and external URLs.
> -> Configuration dialog
> 
> 
> Diffs
> -----
> 
>   plasma/generic/applets/CMakeLists.txt 4ace449 
>   plasma/generic/applets/icon/CMakeLists.txt 1094666 
>   plasma/generic/applets/icon/icon.h 45fa07c 
>   plasma/generic/applets/icon/icon.cpp 8288850 
>   plasma/generic/applets/icon/package/contents/config/main.xml PRE-CREATION 
>   plasma/generic/applets/icon/package/contents/ui/main.qml PRE-CREATION 
>   plasma/generic/applets/icon/package/metadata.desktop PRE-CREATION 
>   plasma/generic/applets/icon/plasma-applet-icon.desktop 8275ddd 
>   plasma/generic/applets/icon/plugin/icon_p.h PRE-CREATION 
>   plasma/generic/applets/icon/plugin/icon_p.cpp PRE-CREATION 
>   plasma/generic/applets/icon/plugin/iconplugin.h PRE-CREATION 
>   plasma/generic/applets/icon/plugin/iconplugin.cpp PRE-CREATION 
>   plasma/generic/applets/icon/plugin/qmldir PRE-CREATION 
> 
> Diff: http://git.reviewboard.kde.org/r/112888/diff/
> 
> 
> Testing
> -------
> 
> 1) Compiles
> 2) Installs
> 3) Loads
> 
> -> Dropping application icon from KickOff - creates icon
> -> Dropping files from Dolphin, both executable and normal files - creates icon
> -> Dropping folder from Dolphin - creates icon applet but neither icon nor text because folder handling is not added yet.
> 
> 4) Opens URL - Clicking on the icon applet opens url with kfmclient
> 
> 
> File Attachments
> ----------------
> 
> icons in panel
>   http://git.reviewboard.kde.org/media/uploaded/files/2013/09/23/snapshot1.png
> icons as launcher
>   http://git.reviewboard.kde.org/media/uploaded/files/2013/09/23/snapshot2.png
> files
>   http://git.reviewboard.kde.org/media/uploaded/files/2013/09/23/snapshot3.png
> folders
>   http://git.reviewboard.kde.org/media/uploaded/files/2013/09/23/snapshot4.png
> 
> 
> Thanks,
> 
> Bhushan Shah
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20130923/13173e72/attachment-0001.html>