--===============5915040122711545201== Content-Type: multipart/alternative; boundary="===============3686749002833311265==" --===============3686749002833311265== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable > On Sept. 23, 2011, 11:37 a.m., Commit Hook wrote: > > This review has been submitted with commit 1f068490c0b431ff05f1965c91e5= d06d7ae3b7f6 by Aaron Seigo to branch master. the patch i committed is significantly different than the one in this revie= w, but the committed patch was motivated/inspired by the submitted patch. t= he layout no longer tries to show the date very every city and instead grou= ps the cities by date. this has a few nice side effects in addition to avoi= ding 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 i= s often the most useful bit of immediate info as in the default configurati= on 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=C3=ADa"), = in another line, always complete. > * removes a that makes the rendering harder > * adds a
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
that makes the rendering harder > * adds a
to include an space between the dates and the event. = > That is, replace = > if (!subText.isEmpty()) { > subText.prepend("
"); subText is never empty as is created wi= th QString("
") > subText.append("
"); = > } > with > subText.append("
"); > = > Of course, kwarning() << data; is not there anymore. > = > = > This addresses bug 260394. > http://bugs.kde.org/show_bug.cgi?id=3D260394 > = > = > 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 na= mes 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 > = > --===============3686749002833311265== Content-Type: text/html; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable
This is an automatically generated e-mail. To reply, visit: http://git.revie= wboard.kde.org/r/102621/

On September 23rd, 2011, 11:37 a.m., Commit= Hook wrote:

This revi=
ew 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 n=
o longer tries to show the date very every city and instead groups the citi=
es by date. this has a few nice side effects in addition to avoiding the wr=
apping issue: it shows less duplicated data, keeps each city in one row, so=
rts 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&#=
39;t show the date on the clock itself.

- Aaron J.


On September 21st, 2011, 7:57 p.m., Jaime Torres Amate wrote:

Review request for Plasma.
By Jaime Torres Amate.

Updated Sept. 21, 2011, 7:57 p.m.

Descripti= on 

The third verion of this patch does:
* Shows the city, UTC displacement, and time in one line and the date (I ho=
pe English people say "on the" where Spanish people say "del=
 d=C3=ADa"), 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.

Testing <= /h1> 
Checked with zero, one and several timezones with short and =
large city names in two machines.
Bugs: 260394

Diffs=

  • libs/plasmaclock/clockapplet.cpp (b1275af)=

View Diff

Screensho= ts

3D"version 3D"showing
--===============3686749002833311265==-- --===============5915040122711545201== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel --===============5915040122711545201==--