<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/118668/">https://git.reviewboard.kde.org/r/118668/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On June 17th, 2014, 8:16 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;">No comments on most of the code change since that looks OK to me :) Besides setToday.
Just a friendly explanation why i did it this way and why i (back then) thought it would be sufficient.
I knew there was going to be a usecase where users of the class would want to know the current day, would want to change it and move back to it. My reasoning for not implementing functionality for that is because startDate could handle it all.
- Want to jump to a date? startDate = new Date(YYYY, MM, DD)
- Want to jump back to the current date? startDate = new Date()
- Want to know the current day? new Date() :)
But i do see now that my naming back then (startDate) is confusing when it allows you to do multiple things. I'm sorry for the hassle you must have went through.
Lastly setToday. I don't understand that one. today() should always return a QDate::currentDate(), right? Why should there be a setter for it? If i understand it correctly you only need this because of the dataengine providing date updates, right? If that's the case then i don't see why i needs to be in this class.. It should be in the QML side as a custom property like so:
Calendar {
id: cal
property date dateEngineDate: somethingFromTheDataEngine
onDateEngineDateChanged: {
// do your magic.. Probably the most part of your current setToday function
}
}
Or i am completely wrong, quite possibly :)</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;">Yeah, the setToday is for when the date changes and/or initializing the date for today. It's better to have one value cached than keeping recreating the date object all the time and pass it back and forth between qml and qt. It's also useful for timezones when eg. your timezone has date 21st but GMT has still 20th. If we're going to add multiple timezone clocks back to the clock applet, you'll need to set different 'today' when you switch the clock. Finally, direct binding for the 'today' means less glue code all around. It's just simpler :)</pre>
<br />
<p>- Martin</p>
<br />
<p>On June 17th, 2014, 10:44 a.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 June 17, 2014, 10:44 a.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;">Basically splits the Calendar::m_startDate into 'today' and 'displayedDate', where displayedDate is the date that is displayed (it controls the days model etc) and can be manipulated by the user by eg. changing months in the plasmoid, and today is the current day, populated by our dataengine (which means it auto-updates with no need for a timer). This allows for greater flexibility and things like "Go back to today" when eg. the plasmoid is hidden or when the user have browsed too far in the calendar and just wants to get back to today (the button to do that pending). Also this fixes a problem where the time dataengine is being polled every 30secs for the clock and would reset the calendar view as the startDate is currently bound to the dataengine and the view resets on that change.</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </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;">All works properly</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/calendar.h <span style="color: grey">(fd2c534)</span></li>
<li>src/declarativeimports/calendar/calendar.cpp <span style="color: grey">(4225579)</span></li>
<li>src/declarativeimports/calendar/qml/MonthMenu.qml <span style="color: grey">(89e9dc2)</span></li>
<li>src/declarativeimports/calendar/qml/MonthView.qml <span style="color: grey">(eee850d)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/118668/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>