Configuring time and date from context menu of digital-clock applet

[Gökçen Eraslan]

Perşembe 28 May 2009 günü (saat 18:54:24) Aaron J. Seigo şunları yazmıştı:
> On Thursday 28 May 2009, Gökçen Eraslan wrote:
> > A patch for $subject is attached. This was the default behaviour in KDE3,
> > now users have to open systemsettings to configure date/time.
>
> the "it was in KDE3" argument is meaningless. either a feature is good and
> desirable, in which case it stands on its own and doesn't need such
> justification, or it isn't a feature we want (for whatever reason(s)) and
> then such justification doesn't matter either.
>

I've said "it was in KDE3" because I see this as some kind of a usability 
regression. I mean, if you can configure time by right clicking the clock 
itself in KDE3 and now, you have to go through Menu->"System Settings"->"Date 
and Time" path to setup time in KDE4, what do you call this? Improvement?

> this is very important because it changes how we think about features. if
> we think of things in terms of "what was in KDE3?" we'll end up replicating
> not just the good things in KDE3 but also the stupid things. and there were
> many stupid things in KDE3, as good as it was. (and yes, i was on occasion
> responsible for some of those stupid things too ;)
>

I didn't say, "we should do this just because it was in KDE3" or "let's put 
all KDE3 features to KDE4", and I think we are smart enough to distinguish 
what was a good feature and should be added to KDE4 and what was not.

> it's ok to use KDE3 as inspiration, just as it is ok to use MacOS, Windows,
> GNOME, BeOS, etc, etc as inspiration. but we don't say "This is the default
> behaviour in MacOS" either, do we? :)
>
> ok, enough philosophy.
>
> > Any reviews, comments?
>
> i wouldn't do it from init(), but inside of contextualActions(). this
> delays the creation of the object, which is good for both start up
> performance and memory usage (at least in the case where the user never
> pulls up a context menu).

OK, I'll change it.

>
> i also wouldn't bother with a createMenu method; it's 3 lines of code. just
> put it in contextualActions:
>
> if (m_actions.isEmpty()) {
>      ... create the actions
> }
>
> also, this line:
>
> KToolInvocation::startServiceByDesktopName("clock", QStringList(), &error);
>
> is just asking for trouble :) it works now, but as soon as another item
> called "clock" appears we get some fun behaviour.
>
> instead, i'd suggest doing a query for the clock kcm from
> KServiceTypeTrader and start the returned service, if any.
>

Actually, this was the way I've seen in Pager applet's context menu. I'll 
change this part of the patch also.

> > PS: By the way, there is a bug about time configuration. By default,
> > update interval of digital-clock plasmoid is 60 seconds when showSeconds
> > is false, and 1 second if it's true. If you change time from clock kcm
> > and showSeconds is false, digital-clock applet refreshes itself 1 minute
> > later. Maybe updateInterval should be hardcoded as 1 second regardless of
> > showSeconds variable.
>
> waking up 60x more often than necessary is a really good way to reduce
> battery life. the clock doesn't update once a minute just because it's lazy
> ;)
>
> as Daniel points out, the correct solution is a dbus signal (probably from
> kded4) that we can listen to.

Yes, you're right. Thanks for review :)

-- 
Gökçen Eraslan
Pardus Developer
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: This is a digitally signed message part.
Url : http://mail.kde.org/pipermail/plasma-devel/attachments/20090528/970d896f/attachment-0001.sig