Review Request: ClockApplet : show seconds in the tooltip

[Aaron Seigo]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://svn.reviewboard.kde.org/r/6336/#review9760
-----------------------------------------------------------


a good step in the right direction, but it's still needs work.

looking at the approach taken, i'd probably do this:

* rename ClockTimer to something clearer -> ClockToolTip?
* have ClockToolTip subclass both QObject and Plasma::ToolTipContent
* move updateToolTip into ClockToolTip, renamed as dataUpdated(const QString &source, const Plasma::DataEngine &data) ;)
* connect to the dataengine on creation of ClockToolTip
* create a ClockToolTip object when toolTipAboutToShow is called
* delete it in toolTipHidden
* pass in the ClockApplet as the parent to ClockToolTip and then in ClockToolTip::dataUpdated, set the mainText and then call Plasma::ToolTipManager::self()->setContent(clockApplet, this)

that means that ClockApplet::updateToolTip would no long be available to subclasses, but that's ok because they were only using it to update the tooltip when the time changed ... which ClockApplet would now be doing for them.


/trunk/KDE/kdebase/workspace/libs/plasmaclock/clockapplet.cpp
<http://svn.reviewboard.kde.org/r/6336/#comment10905>

    this will interfere with any clock applet (which is all of them) which implements dataUpdated.
    
    moreover, calling updateTipContent without the data passed in, only to discard it seems wasteful.


- Aaron


On Jan. 29, 2011, 2:15 p.m., Iamluc wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://svn.reviewboard.kde.org/r/6336/
> -----------------------------------------------------------
> 
> (Updated Jan. 29, 2011, 2:15 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Summary
> -------
> 
> Hi,
> 
> When using a clock applet, seconds are often not visibles. But sometimes you need them.
> This patch shows them in the tooltip of the applet.
> 
> Analog-clock has been updated to refresh the tooltip every seconds.
> If this change is accepted, I could change digital-clock
> 
> Thanks !
> 
> 
> Diffs
> -----
> 
>   /trunk/KDE/kdebase/workspace/libs/plasmaclock/clockapplet.h 1217808 
>   /trunk/KDE/kdebase/workspace/libs/plasmaclock/clockapplet.cpp 1217808 
> 
> Diff: http://svn.reviewboard.kde.org/r/6336/diff
> 
> 
> Testing
> -------
> 
> It Works on plasmoidviewer and with a real session
> 
> 
> Thanks,
> 
> Iamluc
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/plasma-devel/attachments/20110129/8941c15f/attachment.htm