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

[Viranch Mehta]

> On March 13, 2012, 5:32 p.m., David Edmundson wrote:
> > plasma/generic/applets/batterymonitor/contents/ui/PopupDialog.qml, line 66
> > <http://git.reviewboard.kde.org/r/104226/diff/5/?file=53015#file53015line66>
> >
> >     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.
> >     
> >

I have adopted the translation straight from the C++ applet now (except the if-else logic):

if (pluggedIn) {
   if (percent<100) return i18n("%1 (charging)", percent);
   else return i18n("%1 (charged)", percent);
} else {
   return i18n("%1 (discharging)", percent);
}


> On March 13, 2012, 5:32 p.m., David Edmundson wrote:
> > plasma/generic/applets/batterymonitor/contents/ui/PopupDialog.qml, line 101
> > <http://git.reviewboard.kde.org/r/104226/diff/5/?file=53015#file53015line101>
> >
> >     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.
> >

We can achieve completely formatted and translated string only by KLocale::prettyFormatDuration() as far as I know. But since we don't yet have KLocale QML bindings, I have a temporary work around:

var time = new Date(remainingMsec);
var hrs = i18np("1 hour", "%1 hours", time.getUTCHours());
var mins = i18np("1 minute", "%1 minutes", time.getUTCMinutes());
return hrs+", "+mins;


> On March 13, 2012, 5:32 p.m., David Edmundson wrote:
> > plasma/generic/applets/batterymonitor/contents/ui/PopupDialog.qml, line 108
> > <http://git.reviewboard.kde.org/r/104226/diff/5/?file=53015#file53015line108>
> >
> >     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.)

Discarded.
This was done to achieve right alignment to the labels (on the left side). For now, they are all left-aligned due to this change.


- Viranch


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


On March 14, 2012, 5:48 p.m., Viranch Mehta wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/104226/
> -----------------------------------------------------------
> 
> (Updated March 14, 2012, 5:48 p.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/20120314/f0330d7a/attachment-0001.html>