----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/107792/#review23704 -----------------------------------------------------------
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. libs/koproperty/editors/dateedit.cpp <http://git.reviewboard.kde.org/r/107792/#comment18130> Please keep the convention - add (C) here and in other files - Jarosław Staniek On Dec. 18, 2012, 12:03 a.m., Friedrich W. H. Kossebau wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/107792/ > ----------------------------------------------------------- > > (Updated Dec. 18, 2012, 12:03 a.m.) > > > Review request for Calligra, Jarosław Staniek and Adam Pigg. > > > Description > ------- > > 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. > > > Diffs > ----- > > libs/koproperty/CMakeLists.txt 05e7032 > libs/koproperty/DefaultFactory.cpp 976d967 > libs/koproperty/editors/dateedit.h 5c1b542 > libs/koproperty/editors/dateedit.cpp a760fe3 > libs/koproperty/editors/datetimeedit.h f910012 > libs/koproperty/editors/datetimeedit.cpp 046759b > libs/koproperty/editors/timeedit.h 149ee71 > libs/koproperty/editors/timeedit.cpp 11acf75 > > Diff: http://git.reviewboard.kde.org/r/107792/diff/ > > > Testing > ------- > > Seems to work fine in calligra/libs/koproperty/test/propertytest. > > > Thanks, > > Friedrich W. H. Kossebau > >
_______________________________________________ calligra-devel mailing list calligra-devel@kde.org https://mail.kde.org/mailman/listinfo/calligra-devel
