Review Request 114493: Plasmate: Merge plasmoidviewer2 branch into frameworks

[Sebastian Kügler]

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



plasmoidviewer/CMakeLists.txt
<http://git.reviewboard.kde.org/r/114493/#comment32697>

    Please make this org.kde.plasma.plasmoidviewershell, we're moving everything from org.kde.* to org.kde.plasma.*



plasmoidviewer/main.cpp
<http://git.reviewboard.kde.org/r/114493/#comment32698>

    comment is wrong here



plasmoidviewer/main.cpp
<http://git.reviewboard.kde.org/r/114493/#comment32699>

    const
    



plasmoidviewer/main.cpp
<http://git.reviewboard.kde.org/r/114493/#comment32700>

    const



plasmoidviewer/main.cpp
<http://git.reviewboard.kde.org/r/114493/#comment32701>

    const



plasmoidviewer/qmlpackages/shell/contents/views/Desktop.qml
<http://git.reviewboard.kde.org/r/114493/#comment32709>

    This reads a bit weird. Maybe the signal should be called onFormFactorChanged?
    
    (Same for location just below.)



plasmoidviewer/qmlpackages/shell/contents/views/Desktop.qml
<http://git.reviewboard.kde.org/r/114493/#comment32710>

    signal should be "screenshotRequested()"?



plasmoidviewer/qmlpackages/shell/contents/views/Desktop.qml
<http://git.reviewboard.kde.org/r/114493/#comment32711>

    instead of the 8, use theme.largeSpacing /2 here, should give roughly the same value, but is DPI-independent.



plasmoidviewer/qmlpackages/shell/contents/views/Konsole.qml
<http://git.reviewboard.kde.org/r/114493/#comment32712>

    Does this button actually do anything? :)



plasmoidviewer/qmlpackages/shell/contents/views/Konsole.qml
<http://git.reviewboard.kde.org/r/114493/#comment32713>

    Does this button do anything?



plasmoidviewer/qmlpackages/shell/contents/views/SdkButtons.qml
<http://git.reviewboard.kde.org/r/114493/#comment32716>

    here, location() as function call looks weird, make it setLocation()



plasmoidviewer/qmlpackages/shell/contents/views/SdkButtons.qml
<http://git.reviewboard.kde.org/r/114493/#comment32715>

    Left Edge (those are two words), same for the other labels



plasmoidviewer/qmlpackages/shell/contents/views/SdkButtons.qml
<http://git.reviewboard.kde.org/r/114493/#comment32714>

    typo, edget -> edge



plasmoidviewer/view.h
<http://git.reviewboard.kde.org/r/114493/#comment32704>

    QStringLiteral("planar")



plasmoidviewer/view.h
<http://git.reviewboard.kde.org/r/114493/#comment32705>

    QStringLiteral("floating")



plasmoidviewer/view.h
<http://git.reviewboard.kde.org/r/114493/#comment32702>

    setFormFactor(), change* indicates a from-to change, not the case here.
    



plasmoidviewer/view.h
<http://git.reviewboard.kde.org/r/114493/#comment32703>

    setLocation



plasmoidviewer/view.cpp
<http://git.reviewboard.kde.org/r/114493/#comment32706>

    Maybe give the applet name here, so it's easier to spot typos, for example?



plasmoidviewer/view.cpp
<http://git.reviewboard.kde.org/r/114493/#comment32707>

    Give containment name here, see above



plasmoidviewer/view.cpp
<http://git.reviewboard.kde.org/r/114493/#comment32708>

    Can applet->pluginInfo() be invalid here? If that's the case, we'll hit an ASSERT in KPluginInfo, better check.


- Sebastian Kügler


On Dec. 16, 2013, 12:36 p.m., Antonis Tsiapaliokas wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/114493/
> -----------------------------------------------------------
> 
> (Updated Dec. 16, 2013, 12:36 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasmate
> 
> 
> Description
> -------
> 
> This is a merge request for plasmoidviewer2 branch into the frameworks branch.
> 
> Plasmoidviewer2 branch contains some missing features and bug fixes.
> 
> 
> Diffs
> -----
> 
>   plasmoidviewer/CMakeLists.txt 30de8e2 
>   plasmoidviewer/fullview.h 7d4c70d 
>   plasmoidviewer/fullview.cpp 898931a 
>   plasmoidviewer/main.cpp f3cdb22 
>   plasmoidviewer/qmlpackages/containment/Messages.sh c8a9b60 
>   plasmoidviewer/qmlpackages/containment/contents/code/LayoutManager.js 62e149a 
>   plasmoidviewer/qmlpackages/containment/contents/config/main.xml d889999 
>   plasmoidviewer/qmlpackages/containment/contents/ui/AppletAppearance.qml b23e098 
>   plasmoidviewer/qmlpackages/containment/contents/ui/BusyOverlay.qml 33d72c0 
>   plasmoidviewer/qmlpackages/containment/contents/ui/main.qml 6558710 
>   plasmoidviewer/qmlpackages/containment/metadata.desktop 9ddd217 
>   plasmoidviewer/qmlpackages/shell/contents/views/Background.qml PRE-CREATION 
>   plasmoidviewer/qmlpackages/shell/contents/views/Desktop.qml e5bedf3 
>   plasmoidviewer/qmlpackages/shell/contents/views/Konsole.qml PRE-CREATION 
>   plasmoidviewer/qmlpackages/shell/contents/views/SdkButtons.qml PRE-CREATION 
>   plasmoidviewer/view.h 0e2e48c 
>   plasmoidviewer/view.cpp fc008b0 
> 
> Diff: http://git.reviewboard.kde.org/r/114493/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Antonis Tsiapaliokas
> 
>

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