<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/107792/">http://git.reviewboard.kde.org/r/107792/</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;">Good work! Okay to have the property options. I propose to mention their meaning in description of the EditorView class. I did that in the past for other property options.
I see following issue: the date format for delegate is not synchronized with the date format of editor (the same for time an datetime); this is not as simple as it seems to be, please look how that was solved in KexiDateTableEdit/KexiTimeTableEdit/KexiDateTimeTableEdit (with Kexi*Formatter). As you can see I used a line edit there with carefully crafted locale-aware masks, what by the way gives much better keyboard interaction (e.g. Tab key press is not stolen so behaves as expected - moves to another cell in Kexi's table view). Perhaps you could try to alter displayFormat using the formatter but I'd prefer the approach with using line edits and maybe optional calendar popups. Of course koproperty cannot depend on these classes so we would be forced to move needed code to kowidgets or koproperty.
Regarding missing top line, please look how overpainting is performed in ComboBox::paintEvent(). Margins/padding can be improved using Qt style sheets (see ComboBox ctor or IntSpinBox ctor).
I propose to push the patch before fixing the above issues.</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="http://git.reviewboard.kde.org/r/107792/diff/1/?file=100088#file100088line4" style="color: black; font-weight: bold; text-decoration: underline;">libs/koproperty/editors/dateedit.cpp</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="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></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">4</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="cm"> Copyright 2012 Friedrich W. H. Kossebau <kossebau@kde.org></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;">Please keep the convention - add (C) here and in other files</pre>
</div>
<br />
<p>- Jarosław</p>
<br />
<p>On December 18th, 2012, 12:03 a.m., Friedrich W. H. Kossebau 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 Calligra, Jarosław Staniek and Adam Pigg.</div>
<div>By Friedrich W. H. Kossebau.</div>
<p style="color: grey;"><i>Updated Dec. 18, 2012, 12:03 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;">The KoProperty editors for Date, Time & DateTime were disabled from the build, because still relying on Q3 classes.
Attached patch ports them to the current approach with EditorCreatorInterface and ValueDisplayInterface.
It also adds support for some min and max options for Date and Time, but then these options are not yet used.
Still okay to have them, or better remove? If to keep them, where could these options be announced, so they can be used in client code?
I enabled the Calendarbutton for Date and DateTime editors, okay or not?
Nitpick is that the upper line of the cell is overdrawn by the editor widgets, but I have no real idea how to stop that.
Would propose to have this fixed separately, if somebody finds a nice solution.</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;">Seems to work fine in calligra/libs/koproperty/test/propertytest.</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>libs/koproperty/CMakeLists.txt <span style="color: grey">(05e7032)</span></li>
<li>libs/koproperty/DefaultFactory.cpp <span style="color: grey">(976d967)</span></li>
<li>libs/koproperty/editors/dateedit.h <span style="color: grey">(5c1b542)</span></li>
<li>libs/koproperty/editors/dateedit.cpp <span style="color: grey">(a760fe3)</span></li>
<li>libs/koproperty/editors/datetimeedit.h <span style="color: grey">(f910012)</span></li>
<li>libs/koproperty/editors/datetimeedit.cpp <span style="color: grey">(046759b)</span></li>
<li>libs/koproperty/editors/timeedit.h <span style="color: grey">(149ee71)</span></li>
<li>libs/koproperty/editors/timeedit.cpp <span style="color: grey">(11acf75)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/107792/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>