<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://git.reviewboard.kde.org/r/118277/">https://git.reviewboard.kde.org/r/118277/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On May 23rd, 2014, 6:02 p.m. CEST, <b>Mark Gaiser</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Hi Martin,
I would like to suggest a different approach.
1. You want to know the current day (which the model and calendar component don't expose).
2. You want the daysModel data to update when the clock ticks over 12.00 PM into the next day.
My suggestion for 1 is the following:
Expose the _index_ from the daysData model that represents the current day. Note, that index is almost always going to be higher then "today" in the current month. So May 23 will not return 23 due to the last 4 days of april also being in the model. This differs per month obviously so your return index should be <daynumber> + n of last month where n = 4 in this case.
My suggestion for 2 is the following:
Upon Calendar constructing start a timer (in the C++ side) to update the model at midnight. The timer should then call Calendar::updateData which will update everything.
What are your thoughts on this?</pre>
</blockquote>
<p>On May 23rd, 2014, 6:06 p.m. CEST, <b>Mark Gaiser</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Added note: I can implement it to the C++ side if you want.</pre>
</blockquote>
<p>On May 23rd, 2014, 7:58 p.m. CEST, <b>Martin Klapetek</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Thanks for the suggestions
I specifically didn't want to add a timer as there's already a dataengine providing the current date and that's being updated every 30 secs, so adding another timer is just a waste. However this solution is general enough that if you reuse this component, you can replace the dataengine with a timer.
As for exposing index...well, the approach I took here seems more simple and obvious...I'm not sure I want to mess with indexes and do unnecessary computations. We already have a property that's being updated (the dataengine's ["Date"]) and all we need is to hook the rest to it. Which is precisely what I did here :)</pre>
</blockquote>
<p>On May 23rd, 2014, 8:21 p.m. CEST, <b>Mark Gaiser</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">It would be my suggestion to remove the dataengine from this code path and let everything be contained in the Calendar component :)
So the dataengine fires a timer every 30 seconds.. that is 86400 (seconds in a day) / 30 = 2880 timer fires for what exactly? Against just 1 when done in the Calendar c++ side :)
It's your call. If you want to get rid of the dataengine then i will implement those two suggestions for you. Otherwise ship this review request.</pre>
</blockquote>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">> So the dataengine fires a timer every 30 seconds.. that is 86400 (seconds in a day) / 30 = 2880 timer fires for what exactly?
To update your clock applet every minute ;)</pre>
<br />
<p>- Martin</p>
<br />
<p>On May 23rd, 2014, 5:22 p.m. CEST, Martin Klapetek wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('https://git.reviewboard.kde.org/static/rb/images/review_request_box_top_bg.ab6f3b1072c9.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for Plasma.</div>
<div>By Martin Klapetek.</div>
<p style="color: grey;"><i>Updated May 23, 2014, 5:22 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
plasma-framework
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">This is the first part of a fix for updating the calendar properly when date changes (after midnight).
It introduces new property on MonthView - "today". This property is updated by dataengine (or can be a timer too, but we already get signals from dataengine every 30 seconds) and is never updated by the code itself. Furthermore, the DayDelegate is now bound to it, so when "today" changes, the selection rectangle in the calendar should also change. And finally, the selected item in the grid is cleared if MonthView's date property is cleared (which is second part of this patch, to the applet itself).</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>src/declarativeimports/calendar/qml/DayDelegate.qml <span style="color: grey">(552769c)</span></li>
<li>src/declarativeimports/calendar/qml/DaysCalendar.qml <span style="color: grey">(6054a9d)</span></li>
<li>src/declarativeimports/calendar/qml/MonthView.qml <span style="color: grey">(e064f01)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/118277/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>