<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="http://git.reviewboard.kde.org/r/105924/">http://git.reviewboard.kde.org/r/105924/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On August 8th, 2012, 8:30 a.m., <b>John Layt</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;">Sorry, but I disagree. The separate config settings are for specific technical reasons, due to previous user feedback, and for backwards compatibility purposes. We have two config settings for Events and Holidays due to Events turning on Akonadi which many people don't want to use but still want to have Holidays displayed. Until we have a proper solution to the Akonadi problem we have to retain the separate config options. In any case, the solution would be in modifying the behaviour of the GUI, not in changing the backend config.
We used to have separate tick-boxes in the GUI directly mapped to the config options, but I changed the Holidays tick-box to the multiple holidays selection widget and had to retain the backwards compatible behaviour of respecting any existing config setting. You also have the usability issue of people ticking the Holiday options but not the Event box and wondering why the Holidays don't show, you would need to change the GUI to make this obvious.
Also, you seem to have other unrelated code fixes included in the review?</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;">Well, of course we could grey out the holiday selection, if display events is unticked. In the current implementation I think it's awkward that you have to scroll through the entire list and disable every region to hide all events. Maybe a extra tick-box for that really would be better. In this case we could also split up the pim event box into options for every kind of event (todos, events, journals).
Actually I don't understand why there is this holiday setting in the widget at all. This holiday calender needs to be merged into akonadi, so the widget show exactly the same events like i.e. korganizer. We have already a Birthday akonadi ressource. Why not simply create a holiday ressource? I could do that, I suppose. What dou you think?
That unrelated code is from https://git.reviewboard.kde.org/r/105779/ feel free to review it :)</pre>
<br />
<p>- Greg</p>
<br />
<p>On August 8th, 2012, 7:35 a.m., Greg T wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.reviewboard.kde.org/media/rb/images/review_request_box_top_bg.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for Plasma and Anne-Marie Mahfouf.</div>
<div>By Greg T.</div>
<p style="color: grey;"><i>Updated Aug. 8, 2012, 7:35 a.m.</i></p>
<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 patch addresses an usuability issue. The config option 'display events' enables/disables now all events/holidays. The user doesn't distinguish between them and it must be possible to turn off that feature with just one click. </pre>
</td>
</tr>
</table>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>
<a href="http://bugs.kde.org/show_bug.cgi?id=281464">281464</a>
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>libs/plasmaclock/calendar.cpp <span style="color: grey">(75bfc31)</span></li>
<li>libs/plasmaclock/calendartable.h <span style="color: grey">(8678593)</span></li>
<li>libs/plasmaclock/calendartable.cpp <span style="color: grey">(d2b436e)</span></li>
<li>libs/taskmanager/groupmanager.cpp <span style="color: grey">(45c15a9)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/105924/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>