Review Request 127616: Digital clock applet re-write

[Wolfgang Bauer]

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




applets/digital-clock/package/contents/ui/main.qml (line 128)
<https://git.reviewboard.kde.org/r/127616/#comment64251>

    This check is wrong and will cause the clock *never* to display seconds if seconds are part of the timeFormatString.
    The last part should be removed.
    
    Btw, this bug is present in the current applet too.
    To reproduce, set the locale to "C" and enable "Show Seconds", the seconds won't be displayed.
    
    See https://git.reviewboard.kde.org/r/127623/


- Wolfgang Bauer


On April 9, 2016, 5:23 nachm., Daniel Faust wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127616/
> -----------------------------------------------------------
> 
> (Updated April 9, 2016, 5:23 nachm.)
> 
> 
> Review request for Plasma.
> 
> 
> Bugs: 360059
>     https://bugs.kde.org/show_bug.cgi?id=360059
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> -------
> 
> This is mostly a refactoring, removing a lot of update functions and making use of QtQuick's event-driven update logic instead.
> It leads to a lot less updates and a much easier to understand code.
> All the formatting and update functions have been reduced to what's necessary in oder to save some cpu time.
> 
> The layout has been refactored as well leading to some small changes:
> - The time label is now vertically centered without an offset
> - The size of the calendar has been reduced slightly
> - Labels can't have multiple lines in vertical panels any more
> - The timezone and date labels have been reduced to 2/3 of the time label size
> 
> The options in the config dialog have been re-ordered.
> 
> And a new feature has been implemented:
> - Seconds are always shown in the tooltip
> 
> And some more changes:
> - If the dates of time zones differ from the currently active time zone, the full date is shown for these time zones in the tooltip
>   (Currently the full date is shown for time zones if their dates differ from the local time zone)
> - Always use font.pointSize in order to get rid of pointSize vs. pixelSize warnings
> 
> 
> Diffs
> -----
> 
>   applets/digital-clock/package/contents/config/config.qml 877e40c 
>   applets/digital-clock/package/contents/ui/CalendarView.qml 381bddb 
>   applets/digital-clock/package/contents/ui/DigitalClock.qml 02d55a9 
>   applets/digital-clock/package/contents/ui/MonthMenu.qml 170ce62 
>   applets/digital-clock/package/contents/ui/Tooltip.qml 2a8dda9 
>   applets/digital-clock/package/contents/ui/configAppearance.qml fc9a09e 
>   applets/digital-clock/package/contents/ui/configTimeZones.qml 3ac84a7 
>   applets/digital-clock/package/contents/ui/main.qml b86b7fe 
> 
> Diff: https://git.reviewboard.kde.org/r/127616/diff/
> 
> 
> Testing
> -------
> 
> Tested with different configurations.
> 
> 
> File Attachments
> ----------------
> 
> Old clock on the left, new one on the right
>   https://git.reviewboard.kde.org/media/uploaded/files/2016/04/09/d2fd6bfc-2f24-417f-a873-bc0683f6873d__clock-tooltip.png
> Tooltip with multiple time zones, current date differs from local tz date
>   https://git.reviewboard.kde.org/media/uploaded/files/2016/04/09/4edaa035-0e69-4af2-885d-59e311ac5ac2__clock-tooltip-multi-tz.png
> Old config dialog
>   https://git.reviewboard.kde.org/media/uploaded/files/2016/04/09/8ca78bd7-f82d-4cfa-b919-60b713d5c557__clock-config-old.png
> New config dialog
>   https://git.reviewboard.kde.org/media/uploaded/files/2016/04/09/a69b5256-9353-40c8-989b-05575f07c15b__clock-config-new.png
> 
> 
> Thanks,
> 
> Daniel Faust
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20160410/dde5059b/attachment.html>