<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://svn.reviewboard.kde.org/r/5518/">http://svn.reviewboard.kde.org/r/5518/</a>
</td>
</tr>
</table>
<br />
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">I wonder whether it would be better to group countries by continent, as for time zone selection. One reason might be if there were cases where a user's country is not in the list, but a neighbouring country might supply suitable holidays - in this case, grouping by continent would make it easier to find a neighbouring country (bearing in mind that not all other countries will be in the list either).
I don't think the default column heading "Select" is intuitive enough when "Days Off" is displayed alongside. I suspect that many people (myself included) would normally equate "holiday" with "day off". In this case, it isn't obvious what the "Select" column would be for. Having some acquaintance with KDE holiday files, I think I may know what the distinction is, but without knowing KDE holiday files I'd be completely in the dark. In any case, it would be more natural for the "Days Off" column to precede the "Select" column (suitably renamed).
In the documentation in the header, there should be an explanation as to why or why not a developer should choose to display the Language column. It isn't obvious, to me at least, that language would be relevant to choosing a holiday region. If one language was spoken in one region and another was spoken in another region, then I'd expect the holidays to be different, but that would be because it was a different region. And if a single holiday had a different name in one language from another, I'd expect the displayed name to follow the user's locale language, not the holiday file's.
It's a pity that your indent will have to be changed to 2 spaces - it's much more legible using 4 spaces.</pre>
<br />
<div>
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="/r/5518/diff/1/?file=38838#file38838line47" style="color: black; font-weight: bold; text-decoration: underline;">/trunk/KDE/kdepimlibs/kholidays/holidayregionselector.h</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">public:</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">47</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="cm"> * Describes the Selection Status of the Holiday Region.</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Change "the Holiday Region" to "a Holiday Region" would make it easier to understand. </pre>
</div>
<br />
<div>
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="/r/5518/diff/1/?file=38838#file38838line61" style="color: black; font-weight: bold; text-decoration: underline;">/trunk/KDE/kdepimlibs/kholidays/holidayregionselector.h</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">public:</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">61</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="cm"> * display all available features including a serach bar and various details columns, and will</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">serach -> search</pre>
</div>
<br />
<p>- David</p>
<br />
<p>On October 2nd, 2010, 6:21 p.m., John Layt wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://svn.reviewboard.kde.orgrb/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 kdelibs and KDE PIM.</div>
<div>By John Layt.</div>
<p style="color: grey;"><i>Updated 2010-10-02 18:21:32</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;">Implement a standard widget in the KHolidays library for clients to use to allow users to select holiday regions to be displayed. The widget provides a number of modes and features to fit most use case scenarios I can think of:
* A display only mode
* A single Holiday Region selection mode
* A multiple Holiday Region selection mode with optional secondary selection for Days Off / preferred Holiday Region
* A search line
* Ability to modify level of detail displayed when space is limited
Some points to think about:
1) I've implemented Qt Designer plugin support for the widget using a KHolidays specific plugin, but should the Designer support actually be at kdepimlibs level instead, i.e. all kdepimlibs widgets in a single shared plugin?
2) Naming of the widget, enum and api calls
3) The layout of the selection boxes and columns
</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;">Test client implementation done in the Plasma clock/calendar.</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>/trunk/KDE/kdepimlibs/includes/KHolidays/HolidayRegionSelector <span style="color: grey">(PRE-CREATION)</span></li>
<li>/trunk/KDE/kdepimlibs/kholidays/CMakeLists.txt <span style="color: grey">(1180311)</span></li>
<li>/trunk/KDE/kdepimlibs/kholidays/holidayregionselector.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>/trunk/KDE/kdepimlibs/kholidays/holidayregionselector.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>/trunk/KDE/kdepimlibs/kholidays/holidayregionselector.ui <span style="color: grey">(PRE-CREATION)</span></li>
<li>/trunk/KDE/kdepimlibs/kholidays/kholidays.widgets <span style="color: grey">(PRE-CREATION)</span></li>
</ul>
<p><a href="http://svn.reviewboard.kde.org/r/5518/diff/" style="margin-left: 3em;">View Diff</a></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Screenshots </h1>
<div>
<a href="http://svn.reviewboard.kde.org/r/5518/s/523/"><img src="http://svn.reviewboard.kde.org/media/uploaded/images/2010/10/02/HolidaysMultipleSelector_400x100.jpg" style="border: 1px black solid;" alt="Widget example" /></a>
</div>
</td>
</tr>
</table>
</div>
</body>
</html>