[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-pim
Subject:    Re: [Kde-pim] KTimeChooser time editing widget
From:       Michael Leupold <lemma () confuego ! org>
Date:       2009-08-22 9:12:16
Message-ID: h6ocpg$7m4$1 () ger ! gmane ! org
[Download RAW message or body]

Thomas McGuire wrote:

> Hi,
> 
> On Thursday 13 August 2009 01:21:01 you wrote:
>> a while ago I took up Allen's request to make KPIM::KTimeEdit ready for
>> kdelibs. I named the new one KTimeChooser as it seemed more appropriate.
>>  The current outcome can be found here and needs a current trunk to
>>  compile (r1006747, one week ago):
>> http://websvn.kde.org/trunk/playground/libs/ui/ktimewidgets
>> 
>> Please check if you're happy with it.
> 
> I had a quick look at the headers only:
> 
> - The private methods in KTimeEdit and the other classes should probably
> be in
>   the private class

Done, I agree it looks cleaner.

> - KTimeChooser should inherit KComboBox, not QComboBox

I disagree on that. The only things KComboBox provides seem to be text 
rotation and text completion - both of which we don't want.

> - I would say remove the TimeFormatOptions option from the KTimeValidator
>   constructor, as you have a setter for that, see
>   http://doc.trolltech.com/qq/qq13-apis.html#theconveniencetrap

Yeah, I agree as well. It will incur a little overhead if you change it 
after constructing the object (lazy initialization of the regular 
expressions just makes the code look horrible but I may reconsider this :)) 
but it looks a lot nicer.

Regards,
Michael

_______________________________________________
KDE PIM mailing list kde-pim@kde.org
https://mail.kde.org/mailman/listinfo/kde-pim
KDE PIM home page at http://pim.kde.org/
[prev in list] [next in list] [prev in thread] [next in thread]