Review Request 109124: qml port currentappcontrol

[David Edmundson]

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


Looks pretty good.
Couple of pretty common mistakes to sort out.


plasma/netbook/applets/currentappcontrol/package/contents/ui/Task.qml
<http://git.reviewboard.kde.org/r/109124/#comment20942>

    License
    (copy and paste from another plasmoid, and be sure to change the year + name)



plasma/netbook/applets/currentappcontrol/package/contents/ui/Task.qml
<http://git.reviewboard.kde.org/r/109124/#comment20945>

    x * (1/2) is an odd way to write x/2
    



plasma/netbook/applets/currentappcontrol/package/contents/ui/Task.qml
<http://git.reviewboard.kde.org/r/109124/#comment20946>

    Remove commented out code.
    
    



plasma/netbook/applets/currentappcontrol/package/contents/ui/main.qml
<http://git.reviewboard.kde.org/r/109124/#comment20943>

    License



plasma/netbook/applets/currentappcontrol/package/contents/ui/main.qml
<http://git.reviewboard.kde.org/r/109124/#comment20944>

    it's better to do
    
    i18n("%1 running apps", numberOfTasks());
    
    as in some languages the order might be different.
    
    see http://techbase.kde.org/Development/Tutorials/Localization/i18n_Mistakes#Pitfall_.232:_Word_Puzzles
    
    (that's for C++, but the logic still applies)


Also you've ported to QML, you need to update the Messages.sh file in the folder to tell it to include QML files. Otherwise the i18n() commands in QML won't be extracted for translation. On top of that you need to tell it to look through subfolders as you now have a plasma package structure.

Make the replacement below.

 - $XGETTEXT *.cpp -o $podir/plasma_applet_currentappcontrol.pot
 + $XGETTEXT `find . -name \*.qml` -o $podir/plasma_applet_currentappcontrol.pot


- David Edmundson


On Feb. 24, 2013, 11:10 a.m., Greg T wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/109124/
> -----------------------------------------------------------
> 
> (Updated Feb. 24, 2013, 11:10 a.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Description
> -------
> 
> Heya folks,
> This is a qml port of the netbook applet currentappcontrol. It's ready to use, but not fully complete.
> Missing features/bugs:
> - If you click on the title on the task bar, it doesn't show the kwin present windows effect, how to implement that? Is there interest in a 'kwin dataengine' or should I go for a mixed C++/QML applet?
> - there is an annoyance in the task list(present windows fallback): I can set a minimum height, but if I close a task and re-set the minimum height, the popup won't shrink again, leaving an unpleasant empty space; plasma bug?
> - the close/restore buttons don't glow on hover. I couldn't load a theme svg using the IconItem. Is that possible?
> - do you know how to read the size of the containment the widget is contained?
> 
> 
> Diffs
> -----
> 
>   plasma/netbook/applets/currentappcontrol/package/contents/ui/main.qml PRE-CREATION 
>   plasma/netbook/applets/currentappcontrol/package/metadata.desktop PRE-CREATION 
>   plasma/netbook/applets/currentappcontrol/CMakeLists.txt 46178ce3a9351845855e9fccfbc13a9d3946fc95 
>   CMakeLists.txt 6084d7dd7655372506e02abe9f141b73155c5857 
>   plasma/netbook/applets/currentappcontrol/package/contents/ui/Task.qml PRE-CREATION 
> 
> Diff: http://git.reviewboard.kde.org/r/109124/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Greg T
> 
>

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