Review Request 120461: Refactor DigitalClock applet to use multiple labels and states

[Kai Uwe Broulik]

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



File Attachment: Variants in vertical panel - digital-clock2.png
<https://git.reviewboard.kde.org//r/120461/#fcomment270>

    Is it the panel which makes it not look horizontally centered?



applets/digital-clock/package/contents/ui/DigitalClock.qml
<https://git.reviewboard.kde.org/r/120461/#comment47215>

    This could be compacted to one-liners without braces



applets/digital-clock/package/contents/ui/DigitalClock.qml
<https://git.reviewboard.kde.org/r/120461/#comment47216>

    Could this be based on some unit?



applets/digital-clock/package/contents/ui/DigitalClock.qml
<https://git.reviewboard.kde.org/r/120461/#comment47217>

    Is this needed?


- Kai Uwe Broulik


On Okt. 2, 2014, 1:27 nachm., Martin Klapetek wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/120461/
> -----------------------------------------------------------
> 
> (Updated Okt. 2, 2014, 1:27 nachm.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> -------
> 
> Right now the clock applet is using one single label (besides the sizehelper) to fit time, timezone and also date. This has several drawbacks like for example the whole label must use one font size, one eliding etc.
> 
> Putting everything into its own label gives way more flexibility as we can position and size things independently - for example making the time bigger font than the timezone or date and elide long timezone name without omitting the date. The sizing is now also simpler and more robust.
> 
> This patch adds the time and timezone labels into Flow which changes direction based on the layout (in vertical panels layouts things vertically, horizontal panels either vertically or horizontally depending on date being shown or not), the date label is then always appended at the bottom. The reason for dateLabel not being in the Flow is that in horizontal panels we want the date label always go to the bottom and be center aligned and Flow does not support horizontal alignment of its children. Two anchors seems much easier in this case.
> 
> This removes two i18n strings and works around having to add another one (line 352 in the patch) - it's not kosher but I did it so it can still be merged for 5.1 which I would be strongly in favor of. If you decide it should wait for 5.2, I'll add the i18n at the line 352).
> 
> Finally, this adds states for separate handling of vertical and horizontal panels, which has cleaned the code quite a lot from all the "vertical ? complex_a_stuff : complex_b_stuff".
> 
> Sorry it took so long, I kept quickly adding more things to finish it asap until the point where I had to stop and start from scratch. Result is much simpler & much cleaner code.
> 
> 
> Diffs
> -----
> 
>   applets/digital-clock/package/contents/ui/DigitalClock.qml 4853716 
> 
> Diff: https://git.reviewboard.kde.org/r/120461/diff/
> 
> 
> Testing
> -------
> 
> I did try a thorough testing of all the features in panels on all screen edges, all seems good.
> 
> 
> File Attachments
> ----------------
> 
> Variants in horizontal panel
>   https://git.reviewboard.kde.org/media/uploaded/files/2014/10/02/30deb86a-4ea6-45b7-b2ed-cfbab0c76e68__digital-clock1.png
> Variants in vertical panel
>   https://git.reviewboard.kde.org/media/uploaded/files/2014/10/02/4fd5dca9-2e90-419e-a60d-f998b8b2bd7e__digital-clock2.png
> 
> 
> Thanks,
> 
> Martin Klapetek
> 
>

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