----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/101575/#review4509 -----------------------------------------------------------
Looks good already, just a couple of API nitpicks. kdeui/widgets/kdatecombobox.h <http://git.reviewboard.kde.org/r/101575/#comment3831> Hm, IIRC you should have the full enum name here, otherwise you won't be able to make a signal connectable to this slot. Besides I'm not sure it makes sense to make a slot out of this one. I'd expect the usage pattern to be mostly about calling that once after creation. kdeui/widgets/kdatecombobox.h <http://git.reviewboard.kde.org/r/101575/#comment3832> What's the default value you're referring to? What about using an invalid QDate for the reset, and then have only the setter? kdeui/widgets/kdatecombobox.h <http://git.reviewboard.kde.org/r/101575/#comment3833> Same comment than for minimum. kdeui/widgets/kdatetimeedit.h <http://git.reviewboard.kde.org/r/101575/#comment3834> Same comments than for KDateComboBox::setOptions kdeui/widgets/kdatetimeedit.h <http://git.reviewboard.kde.org/r/101575/#comment3835> Same comment than for KDateComboBox kdeui/widgets/kdatetimeedit.h <http://git.reviewboard.kde.org/r/101575/#comment3836> See other comments I added on reset*() kdeui/widgets/ktimecombobox.h <http://git.reviewboard.kde.org/r/101575/#comment3837> Always the same old song. (see other setOptions comments) ;-) - Kevin On June 10, 2011, 9:18 p.m., John Layt wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/101575/ > ----------------------------------------------------------- > > (Updated June 10, 2011, 9:18 p.m.) > > > Review request for kdelibs, KDEPIM, KPhotoAlbum, Skrooge, Zanshin, Kevin > Ottens, and David Jarvie. > > > Summary > ------- > > [Sorry this is a post-commit review and took so long to remember to post. Bad > coder, no cookie for you!] > > This review is for some new replacement widgets for the popular KDEPIM > KDateEdit and KTimeEdit widgets which were copied into a number of other > projects. These new widgets are clean rewrites (the original widgets have > history back to KDE2 days) with slightly changed api but otherwise should > replicate the same functionality with a couple of new features. They will be > available for use by any apps using kdelibs 4.7. > > The 3 new widgets are: > > KDateComboBox: A date entry widget derived from KComboBox, the drop-down menu > can display a date picker and list of "fancy" dates to choose from. The list > of dates can be configured. > > KTimeComboBox: A time entry widget derived from KComboBox, the drop-down menu > can display a list of times to choose from. The list of times can be > configured. > > KDateTimeEdit: A KDateTime entry widget combining KDateComboBox and > KTimeComboBox with optional combo's to select the calendar system and time > spec as well. This widget should only be used if you want time spec aware > data entry. > > All widgets can accept a null or invalid input, it is up to the coder to > check for validity of input using isValid() if required. All feature options > of the widgets can be configured. All widgets can optionally display a > warning box on focus out if the entry is invalid. All widgets can be used in > Qt Designer. > > I'm particularly looking for input on the api, and what QWidget event virtual > methods I should be reimplementing to make the classes BIC future-proof. > > > Diffs > ----- > > kdeui/CMakeLists.txt 1e8b259 > includes/KDateComboBox PRE-CREATION > includes/KDateTimeEdit PRE-CREATION > includes/KTimeComboBox PRE-CREATION > includes/CMakeLists.txt 7a8bc5c > kdeui/tests/CMakeLists.txt c7b8026 > kdeui/tests/kdatecomboboxtest.h PRE-CREATION > kdeui/tests/kdatecomboboxtest.cpp PRE-CREATION > kdeui/tests/kdatetimeedittest.h PRE-CREATION > kdeui/tests/kdatetimeedittest.cpp PRE-CREATION > kdeui/tests/ktimecomboboxtest.h PRE-CREATION > kdeui/tests/ktimecomboboxtest.cpp PRE-CREATION > kdeui/widgets/kdatecombobox.h PRE-CREATION > kdeui/widgets/kdatecombobox.cpp PRE-CREATION > kdeui/widgets/kdatetimeedit.h PRE-CREATION > kdeui/widgets/kdatetimeedit.cpp PRE-CREATION > kdeui/widgets/kdatetimeedit.ui PRE-CREATION > kdeui/widgets/ktimecombobox.h PRE-CREATION > kdeui/widgets/ktimecombobox.cpp PRE-CREATION > kdewidgets/kde.widgets 9040538 > > Diff: http://git.reviewboard.kde.org/r/101575/diff > > > Testing > ------- > > Unit tests written for non-gui functionality. Gui functionality tested in Qt > Designer. KDateTimeEdit still has a couple of minor bugs, but I didn't want > to hold the review up any longer. > > > Screenshots > ----------- > > Qt Designer Preview > http://git.reviewboard.kde.org/r/101575/s/180/ > KDateComboBox > http://git.reviewboard.kde.org/r/101575/s/181/ > KTimeComboBox > http://git.reviewboard.kde.org/r/101575/s/182/ > > > Thanks, > > John > >
