Discussion:
Review Request: Proper size for analog clock timezone label
Alain Boyer
2010-06-21 05:16:52 UTC
Permalink
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.kde.org/r/4418/
-----------------------------------------------------------

Review request for Plasma.


Summary
-------

This patch adjusts the timezone label of the analog clock to the width of the city text. Although this is a cosmetic change that is very subjective, I find that the overall look, when multiple clocks are horizontally aligned next to each other, is nicer and more polished.

This is a very minor change, but since I have been away from KDE hacking for a while and we are so close to the next release, I figured I'd put it up for review.


Diffs
-----

/trunk/KDE/kdebase/workspace/plasma/generic/applets/analog-clock/clock.h 1140389
/trunk/KDE/kdebase/workspace/plasma/generic/applets/analog-clock/clock.cpp 1140389

Diff: http://reviewboard.kde.org/r/4418/diff


Testing
-------

Works just fine in plasmoidviewer.


Screenshots
-----------

Before
http://reviewboard.kde.org/r/4418/s/440/
After
http://reviewboard.kde.org/r/4418/s/441/


Thanks,

Alain
Aaron Seigo
2010-06-21 11:47:41 UTC
Permalink
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.kde.org/r/4418/#review6211
-----------------------------------------------------------

Ship it!


i quite like the improved look, there is one change that needs to be made noted below, but then it can go in.


/trunk/KDE/kdebase/workspace/plasma/generic/applets/analog-clock/clock.cpp
<http://reviewboard.kde.org/r/4418/#comment5800>

in the case of a small analog clock with a long city name, this will result in returning a rect that is wider than the widget.

so width needs to be bounded to rect.width().


- Aaron
Post by Alain Boyer
-----------------------------------------------------------
http://reviewboard.kde.org/r/4418/
-----------------------------------------------------------
(Updated 2010-06-21 05:16:47)
Review request for Plasma.
Summary
-------
This patch adjusts the timezone label of the analog clock to the width of the city text. Although this is a cosmetic change that is very subjective, I find that the overall look, when multiple clocks are horizontally aligned next to each other, is nicer and more polished.
This is a very minor change, but since I have been away from KDE hacking for a while and we are so close to the next release, I figured I'd put it up for review.
Diffs
-----
/trunk/KDE/kdebase/workspace/plasma/generic/applets/analog-clock/clock.h 1140389
/trunk/KDE/kdebase/workspace/plasma/generic/applets/analog-clock/clock.cpp 1140389
Diff: http://reviewboard.kde.org/r/4418/diff
Testing
-------
Works just fine in plasmoidviewer.
Screenshots
-----------
Before
http://reviewboard.kde.org/r/4418/s/440/
After
http://reviewboard.kde.org/r/4418/s/441/
Thanks,
Alain
Loading...