Differential D17467

digital-clock: Show events on initial expansion for all time zones
Needs ReviewPublic
Actions

Authored by mbehrendt on Dec 10 2018, 7:49 AM.

Details

Reviewers
plasma-devel
Group Reviewers
Plasma
Summary

When expanding the digital-clock applet the holidayList is not updated, so that events for the current day are not shown.
In order to show the events for the current day after expansion one had to click on another day in the month view and then click back to the current day.

This patch fixes that by properly updating the holidaysList's model.

The problem was that in "onAgendaUpdated" two date objects were compared.
But one object had a different time set based on the current local time zone so that the holidaysList never got updated for time zones other than GMT+0000.

Diff Detail

Repository
R120 Plasma Workspace
Lint
Lint Skipped
Unit
Unit Tests Skipped
mbehrendt created this revision.Dec 10 2018, 7:49 AM
Restricted Application added a project: Plasma. · View Herald TranscriptDec 10 2018, 7:49 AM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
mbehrendt requested review of this revision.Dec 10 2018, 7:49 AM
broulik added a subscriber: broulik.Dec 10 2018, 8:48 AM
broulik added inline comments.
applets/digital-clock/package/contents/ui/CalendarView.qml
55

I think this should check for isExpanded so it does not update the events (potentially heavy operation) when *closing*

mbehrendt updated this revision to Diff 47248.Dec 10 2018, 9:07 AM

Besides checking for isExpanded this prevents from updating the holidaysList twice by checking if the date needs to be reset.

mbehrendt updated this revision to Diff 47252.Dec 10 2018, 9:15 AM

This should even be lighter

mbehrendt updated this revision to Diff 47255.Dec 10 2018, 9:30 AM

only once set hasExpanded = true

mbehrendt updated this revision to Diff 47256.Dec 10 2018, 9:34 AM
mbehrendt marked an inline comment as done.

fix typos

mbehrendt updated this revision to Diff 47317.Dec 10 2018, 8:53 PM

more simplifications

mart added a subscriber: mart.Dec 11 2018, 2:27 PM
mart added inline comments.
applets/digital-clock/package/contents/ui/CalendarView.qml
60

instead of storing a local hasExpanded, could you just check for holidaysList.model not being null?

mbehrendt updated this revision to Diff 47407.Dec 12 2018, 1:31 AM

Total overhaul of the patch.
I think I found the root cause for the events not being displayed.
I also inserted some if clauses to reduce calculations.
I also removed setting holidaysList.model = null as I could not reproduce the issue that was mentioned in the comment.

mbehrendt updated this revision to Diff 47409.Dec 12 2018, 2:00 AM

remove local variable "currentDate" and use "getDate" instead of "getDay"

mbehrendt retitled this revision from digital-clock: Show events on expanding applet to digital-clock: Show events on initial expansion for all time zones.Dec 12 2018, 2:07 AM
mbehrendt edited the summary of this revision. (Show Details)
mbehrendt marked an inline comment as done.Dec 12 2018, 2:10 AM
mbehrendt added inline comments.
applets/digital-clock/package/contents/ui/CalendarView.qml
60

No, the model initially seems to be an empty list.

mart added inline comments.Dec 13 2018, 10:22 AM
applets/digital-clock/package/contents/ui/CalendarView.qml
60

another way you can try is to do it on Component.onCompleted: { }

mbehrendt added inline comments.Dec 13 2018, 11:03 AM
applets/digital-clock/package/contents/ui/CalendarView.qml
60

Yes I laready considered that. But when I tried your suggestion, I figured out what the real problem was that the events are initially not shown. So there is no need to manually initialize the holidaysList. According to that I updated the revision and diff.

Revision Contents
Changeset List

Diff 47409

View Options

applets/digital-clock/package/contents/ui/CalendarView.qml

Loading...