Review Request: do not split the year in the clock tip

Aaron J. Seigo aseigo at kde.org
Fri Sep 23 11:40:32 UTC 2011



> On Sept. 23, 2011, 11:37 a.m., Commit Hook wrote:
> > This review has been submitted with commit 1f068490c0b431ff05f1965c91e5d06d7ae3b7f6 by Aaron Seigo to branch master.

the patch i committed is significantly different than the one in this review, but the committed patch was motivated/inspired by the submitted patch. the layout no longer tries to show the date very every city and instead groups the cities by date. this has a few nice side effects in addition to avoiding the wrapping issue: it shows less duplicated data, keeps each city in one row, sorts timezones by their time and puts the date at the top which is often the most useful bit of immediate info as in the default configuration we don't show the date on the clock itself.


- Aaron J.


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/102621/#review6762
-----------------------------------------------------------


On Sept. 21, 2011, 7:57 p.m., Jaime Torres Amate wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/102621/
> -----------------------------------------------------------
> 
> (Updated Sept. 21, 2011, 7:57 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Summary
> -------
> 
> The third verion of this patch does:
> * Shows the city, UTC displacement, and time in one line and the date (I hope English people say "on the" where Spanish people say "del día"), in another line, always complete.
> * removes a <table> that makes the rendering harder
> * adds a <br> to include an space between the dates and the event. 
> 
> Look at the screnshot.
> 
> In any case, I think one part of this patch MUST be commited.
> * removes a <table> that makes the rendering harder
> * adds a <br> to include an space between the dates and the event. 
> That is, replace 
> if (!subText.isEmpty()) {
>      subText.prepend("<table>");		subText is never empty as is created with QString("<table>")
>      subText.append("</table>");		
>    }
> with
> subText.append("</table><br>");
> 
> Of course, kwarning() << data; is not there anymore.
> 
> 
> This addresses bug 260394.
>     http://bugs.kde.org/show_bug.cgi?id=260394
> 
> 
> Diffs
> -----
> 
>   libs/plasmaclock/clockapplet.cpp b1275af 
> 
> Diff: http://git.reviewboard.kde.org/r/102621/diff
> 
> 
> Testing
> -------
> 
> Checked with zero, one and several timezones with short and large city names in two machines.
> 
> 
> Screenshots
> -----------
> 
> version 2
>   http://git.reviewboard.kde.org/r/102621/s/262/
> showing also the UTC displacement
>   http://git.reviewboard.kde.org/r/102621/s/267/
> 
> 
> Thanks,
> 
> Jaime Torres
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20110923/94770032/attachment.html>


More information about the Plasma-devel mailing list