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

[Sebastian Kügler]

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

Ship it!


Nice work!


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

    There may be a corner case here, when the formfactor changes to Planar or MediaCenter (i.e. 
    "other"), Layout.* properties may still be set.
    
    I don't know if this is much of an issue in practise, as that kind of formfactor change should happen very rarely, but might be something to improve (in the future).


- Sebastian Kügler


On Oct. 2, 2014, 1:27 p.m., Martin Klapetek wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/120461/
> -----------------------------------------------------------
> 
> (Updated Oct. 2, 2014, 1:27 p.m.)
> 
> 
> 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/254e55d6/attachment.html>