Kronometer now in KDE Review

[Elvis Angelaccio]

2014-04-17 20:04 GMT+02:00 Ingo Klöcker <kloecker at kde.org>:

> On Wednesday 16 April 2014 12:56:01 Elvis Angelaccio wrote:
> > So, the choice is between the branch <test> and <test2>. Let me know
> > what do you prefer.
> >
> > For completeness the alternatives in details are:
> >
> > - branch <test>: no splitters, with dividers (i.e. 4 QFrames in a
> > horizontal layout):
> > http://abload.de/img/kronometer-no-splittevdklc.png
>
> This would look much better if you remove the ':' and '.' after the
> numbers. IMO the ':'/'.' are superfluous because the numbers are already
> clearly separated by the frame border.
>
> Also you should probably right-align the numbers. In particular, the
> hours. For the other numbers alignment probably doesn't matter.
> Moreover, I think it would look best if all four frames were the same
> size.
>
> Hi, thanks for your suggestions!
I've committed all these changes on the <test> branch.
I've applied the right alignment to all the labels because looks more
consistent. (e.g when the window is full size).


>
> > - branch <test2>:
> > no splitters, no dividers (i.e. single QFrame with a grid layout):
> > http://abload.de/img/kronometer-no-divideri4kce.png
>
> Here the ':'/'.' need to stay for obvious reasons. But you should try to
> make the spacing between the numbers and the ':'/'.' identical. Possible
> solution: Put the ':'/'.' into columns of their own. And right-align the
> hours.
>
>
This one is not trivial. I could use columns on their own for the
"dividers", but these symbols are displayed only if there are numbers on
their right.
At the moment I handle this check in the QTimeFormat class, it would be
difficult to move this logic to a widget class like QTimeDisplay is.

Since also Albert agrees with the first alternative, probably the second
one it's not worth of the effort.


>
> I'd also get rid of the upper toolbar. You do already have the essential
> tools in the lower toolbar and having two toolbars even with differently
> sized icons makes the UI look unnecessarily crowded.
>
>
Final screenshot with all these changes:
http://abload.de/img/kronometerdrkjq.png



>
> Just my two cents.
>
>
> Regards,
> Ingo
>

Regards,
Elvis
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20140418/39622629/attachment.htm>