Review Request: Add tooltips to show current song when nowplaying is in the panel
Alex Merry
kde at randomguy3.me.uk
Sun Dec 14 18:10:43 CET 2008
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.vidsolbach.de/r/309/#review296
-----------------------------------------------------------
Ship it!
Apart from the comments below, it looks good to me.
I'm happy for it to go in providing the translators are happy about the extra string (kde-i18n@, I believe is the mailing list you want).
You could get around the string freeze by commenting out line 301 with a FIXME: 4.3 comment, and putting
toolTip.setSubText(m_artist);
instead for now. I would grab aacid on irc and ask him (or email him @kde.org).
/trunk/KDE/kdeplasma-addons/applets/nowplaying/nowplaying.cpp
<http://reviewboard.vidsolbach.de/r/309/#comment244>
What happens if you Feeling Good by Michael Bublé playing, then the next song is Muse's cover of the same track?
/trunk/KDE/kdeplasma-addons/applets/nowplaying/nowplaying.cpp
<http://reviewboard.vidsolbach.de/r/309/#comment245>
How is "@info:tooltip" a useful comment?
Translators need to know what on earth "by" means in this context.
How about "song artist, displayed below the song title"?
- Alex
On 2008-12-14 09:09:14, Tony Murray wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.vidsolbach.de/r/309/
> -----------------------------------------------------------
>
> (Updated 2008-12-14 09:09:14)
>
>
> Review request for Plasma and Alex Merry.
>
>
> Summary
> -------
>
> This adds a tooltip when on the panel. It includes artwork and track title and artist. It does add one string, so we would have to clear that with translators if we add it now "by %1".
>
> It does not update the tooltip while it is show, but the timeout is rather short so it might not be worth the extra cpu cycles.
>
> QPixmap.scale handles null pixmaps gracefully, so it isn't necessary to have a check for that right?
>
>
> Diffs
> -----
>
> /trunk/KDE/kdeplasma-addons/applets/nowplaying/nowplaying.h
> /trunk/KDE/kdeplasma-addons/applets/nowplaying/nowplaying.cpp
>
> Diff: http://reviewboard.vidsolbach.de/r/309/diff
>
>
> Testing
> -------
>
> On my local svn machine only. Vertical, Horizontal, and Planar all tested. (It isn't supposed to show for Planar ;)
>
>
> Thanks,
>
> Tony
>
>
More information about the Plasma-devel
mailing list