Review Request 115431: Fix the numbers cut off problem in digital clock applet

Yichao Zhou broken.zhou at gmail.com
Tue Feb 4 03:58:33 UTC 2014



> On Feb. 3, 2014, 9:32 a.m., Sebastian Kügler wrote:
> > plasma/generic/applets/digital-clock/clock.cpp, line 714
> > <https://git.reviewboard.kde.org/r/115431/diff/3/?file=241362#file241362line714>
> >
> >     This seems overly complex to me. In other such places, we happily use "M".
> >     
> >     Do you still see the issue if you just use "M" here?
> 
> Yichao Zhou wrote:
>     I'm not quite understood.  Do you mean that we should change everything to M?  For example, "06:00 PM Saturday" to "MMMMMMMMMMMMMMMMM"?  I think that will be overly wide.
> 
> Sebastian Kügler wrote:
>     Well, the number of characters should be the timeformat's numer, but it makes sense to use the same letter for all of them, because we don't want these metrics to change when the time changes, this will make the clock jump whenever the time changes, and possibly even the panel (since the clock's applet size depends on the size of the rendered text). (An issue, I've just fixed not long ago in Plasma Next.)

I think it is more reasonable to change from
"06:00 PM Saturday" to
"00:00 MM Mmmmmmmm", rather than
"MMMMMMMMMMMMMMMMM".

Because some fonts's "M"s are almost twice as wide as "0"s and ":"s.  Using only "M" will make the "fakeWidth" be twice as much as the "realWidth".  I think this will make it a little ugly.  So I choose to use the second string.

In order to let these metrics not to change when the time changes TOTALLY, I think this is very hard:  You need to first use a fakeString to calculate the width and the positions of each character and then manually draw each character on each precomputed position (because the font may not be a monospace font).  I think using "M" here cannot fix this issue (the result is same).


- Yichao


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/115431/#review48795
-----------------------------------------------------------


On Feb. 2, 2014, 12:31 p.m., Yichao Zhou wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/115431/
> -----------------------------------------------------------
> 
> (Updated Feb. 2, 2014, 12:31 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Bugs: 228902
>     http://bugs.kde.org/show_bug.cgi?id=228902
> 
> 
> Repository: kde-workspace
> 
> 
> Description
> -------
> 
> For bug, see
> https://bugs.launchpad.net/ubuntu/+source/kde-workspace/+bug/1052059 and
> https://bugs.kde.org/show_bug.cgi?id=228902
> 
> The problem is that in original code, it always uses 23:59 to calculate the width of the text.  However, in some font, AM is wider than PM, and 0 is wider than 2 and 5.  That causes some fonts been cropped.
> 
> 
> Diffs
> -----
> 
>   plasma/generic/applets/digital-clock/clock.cpp f26e328 
> 
> Diff: https://git.reviewboard.kde.org/r/115431/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Yichao Zhou
> 
>

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


More information about the Plasma-devel mailing list