Review Request: Merge the final and fixed QML battery monitor to master.

David Edmundson kde at davidedmundson.co.uk
Tue Mar 13 17:32:10 UTC 2012


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


Read up on i18n, ideally most of http://techbase.kde.org/Development/Tutorials/Localization/ and double check everything again.

Also personally I like to submit a screenshot with any very visual change.


plasma/generic/applets/batterymonitor/contents/ui/PopupDialog.qml
<http://git.reviewboard.kde.org/r/104226/#comment9094>

    This is not translated.



plasma/generic/applets/batterymonitor/contents/ui/PopupDialog.qml
<http://git.reviewboard.kde.org/r/104226/#comment9093>

    This i18n string doesn't really work.
    
    1) This string doesn't really contain any words, so it's not really suitable for translation. At least use i18nc() so translators have context of what it is.
    
    2) the state (i.e charging, charged, discharging) is never translated.
    
    



plasma/generic/applets/batterymonitor/contents/ui/PopupDialog.qml
<http://git.reviewboard.kde.org/r/104226/#comment9095>

    This isn't translated.
    
    Also this is a word puzzle.
    
    http://techbase.kde.org/Development/Tutorials/Localization/i18n_Mistakes#Pitfall_.232:_Word_Puzzles
    
    You also can't do
    
    if (hrs==1) {
     "hour"
    } else {
     "hours"
    }
    for some languages plurals come after 1st, 11th 111th.. it's not as simple as you just wrote.
    
    use i18np.
    



plasma/generic/applets/batterymonitor/contents/ui/PopupDialog.qml
<http://git.reviewboard.kde.org/r/104226/#comment9096>

    Don't do this to determine how wide something should be.
    What if the japanese for "power management enabled" is only 3 characters long and the time remaining is larger?
    
    Even if you could garauntee it's the longest string right now, what if someone changes this in the future?
    
    set the Grid to be 
    width:childRect.width.
    
    and remove the call to width on all these labels, and that /should/ work. (I've not tested that and could be wrong.)


- David Edmundson


On March 13, 2012, 11:59 a.m., Viranch Mehta wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/104226/
> -----------------------------------------------------------
> 
> (Updated March 13, 2012, 11:59 a.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Description
> -------
> 
> I fixed the QML battery monitor to be fairly usable and this diff merges it to master.
> 
> 
> Diffs
> -----
> 
>   plasma/generic/applets/CMakeLists.txt 2dedcb2 
>   plasma/generic/applets/batterymonitor/CMakeLists.txt PRE-CREATION 
>   plasma/generic/applets/batterymonitor/Messages.sh PRE-CREATION 
>   plasma/generic/applets/batterymonitor/README.txt PRE-CREATION 
>   plasma/generic/applets/batterymonitor/battery-oxygen-inkscape.svgz PRE-CREATION 
>   plasma/generic/applets/batterymonitor/battery-oxygen.svgz PRE-CREATION 
>   plasma/generic/applets/batterymonitor/contents/config/main.xml PRE-CREATION 
>   plasma/generic/applets/batterymonitor/contents/ui/IconButton.qml PRE-CREATION 
>   plasma/generic/applets/batterymonitor/contents/ui/PopupDialog.qml PRE-CREATION 
>   plasma/generic/applets/batterymonitor/contents/ui/batterymonitor.qml PRE-CREATION 
>   plasma/generic/applets/batterymonitor/contents/ui/config.ui PRE-CREATION 
>   plasma/generic/applets/batterymonitor/metadata.desktop PRE-CREATION 
>   plasma/generic/dataengines/powermanagement/powermanagementengine.h 20642c2 
>   plasma/generic/dataengines/powermanagement/powermanagementengine.cpp 5572fcb 
>   plasma/generic/dataengines/powermanagement/powermanagementjob.h 2c32015 
>   plasma/generic/dataengines/powermanagement/powermanagementjob.cpp e205bb0 
>   plasma/generic/dataengines/powermanagement/powermanagementservice.operations ad1301f 
> 
> Diff: http://git.reviewboard.kde.org/r/104226/diff/
> 
> 
> Testing
> -------
> 
> Applet and dataengine both tested and work fine.
> 
> 
> Thanks,
> 
> Viranch Mehta
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20120313/3337f5b0/attachment.html>


More information about the Plasma-devel mailing list