D12536: [timer applet] Change how time is tracked

David Edmundson noreply at phabricator.kde.org
Thu Apr 26 09:13:31 UTC 2018


davidedmundson requested changes to this revision.
davidedmundson added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> TimerView.qml:54
>          id: t;
> -        interval: 1000;
> +        interval: 20;
>          onTriggered: {

that's a lot of wakeups. Do you really need to do this?

> TimerView.qml:56
>          onTriggered: {
> -            if (root.seconds != 0) {
> -                root.seconds--;
> +            var ts = new Date().getTime();
> +            if (root.seconds > 0) {

There's a behavioural change not mentioned in the commit message :/

If I have a timer for 5 minutes and suspend after 1 minute for an hour    in the old code when we resume we will see a timer for 4 minutes left. In the new code we will see the timer has finished.

You can make an argument this is better, but it shouldn't be accidental. 
It's also not immune to timezone changes; we'd want this always in GMT.

*if* we want this behaviour the clock dataengine might be a good solution, that aligns itself to seconds automatically.
If we don't, maybe we want to neatly expose a QElapsedTimer; possibly updating a property on every QQuickWindow::beforeSyncronisation.

REPOSITORY
  R114 Plasma Addons

REVISION DETAIL
  https://phabricator.kde.org/D12536

To: mmazur, #plasma, davidedmundson
Cc: davidedmundson, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20180426/f2495079/attachment-0001.html>


More information about the Plasma-devel mailing list