D17464: [Timer applet] Minor fixes for the applet

[Alexander Kernozhitsky]

gepardo created this revision.
Herald added a project: Plasma.
Herald added a subscriber: plasma-devel.
gepardo requested review of this revision.

REVISION SUMMARY
BUG: 395182
BUG: 361025

Here are the changes made:

- Allow Timer widget to work well on panels, both vertical and horizontal.

- Use root.digitHasChanged(); instead of main.digitChanged(); This line was changed before in https://phabricator.kde.org/D12534, but it seems that main.digitChanged(); doesn't exist also.

- Use "expireTimeout" instead of "timeout", as explained in https://bugs.kde.org/show_bug.cgi?id=361025.

- Increase notification time from 2s to 5s, so it's harder to miss it.

TEST PLAN
Tried placing the timer on a panel, then tried to resize the panel. Tested on both vertical and horizontal panels.

I regularly use this applet on a vertical panel without issues.

REPOSITORY
R114 Plasma Addons

BRANCH
timer-on-panel-fix (branched from master)

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

AFFECTED FILES
applets/timer/package/contents/ui/TimerDigit.qml
applets/timer/package/contents/ui/TimerView.qml
applets/timer/package/contents/ui/main.qml

To: gepardo
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

[Alexander Kernozhitsky]

gepardo updated this revision to Diff 47224.
gepardo added a comment.


Add a comment about value 0.77

REPOSITORY
R114 Plasma Addons

CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D17464?vs=47219&id=47224

BRANCH
timer-on-panel-fix (branched from master)

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

AFFECTED FILES
applets/timer/package/contents/ui/TimerDigit.qml
applets/timer/package/contents/ui/TimerView.qml
applets/timer/package/contents/ui/main.qml

To: gepardo, muhlenpfordt, mmazur, friedreich, #plasma
Cc: davidedmundson, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

[David Edmundson]

davidedmundson added a comment.


Cool.

INLINE COMMENTS

TimerView.qml:46
+ states: [
+ State {
+ name: "verticalPanel"

what if it's neither (i.e on the desktop) ?

TimerView.qml:53
+ Layout.fillWidth: true
+ Layout.maximumHeight: (2 * width / digits) * (root.showTitle ? 2 : 1)
+ height: Layout.maximumHeight

never set size hints from current size, you're asking for binding loops.

setting it based on the implicitWith is ok

TimerView.qml:65
+ Layout.fillWidth: false
+ Layout.minimumWidth: height / 2 * digits * 0.77
+ Layout.maximumWidth: Layout.minimumWidth

where has 0.77 come from

REPOSITORY
R114 Plasma Addons

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

To: gepardo, muhlenpfordt, mmazur, friedreich, #plasma
Cc: davidedmundson, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

[Alexander Kernozhitsky]

gepardo added inline comments.

INLINE COMMENTS

davidedmundson wrote in TimerView.qml:46
what if it's neither (i.e on the desktop) ?

Doesn't it use the default sizing settings (as used before this patch)?

REPOSITORY
R114 Plasma Addons

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

To: gepardo, muhlenpfordt, mmazur, friedreich, #plasma
Cc: davidedmundson, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart