Re: [PATCHv6 3/8] gitweb: right-align date cell in shortlog

From: Jakub Narebski <jnareb@gmail.com>
Date: Jun 27, 2009, 12:14 UTC
Message-ID: <200906271414.53717.jnareb@gmail.com>
In-Reply-To: <cb7bb73a0906261106n5e12948dydd02bd8d1b19a5e6@mail.gmail.com>

On Fri, 26 June 2009, Giuseppe Bilotta wrote:

Show 12 quoted lines

> 2009/6/26 Jakub Narebski <jnareb@gmail.com>:
>> On Thu, 25 June 2009, Giuseppe Bilotta wrote:
>>
>>> diff --git a/gitweb/gitweb.css b/gitweb/gitweb.css
>>> index 68b22ff..7240ed7 100644
>>> --- a/gitweb/gitweb.css
>>> +++ b/gitweb/gitweb.css
>>> @@ -180,6 +180,10 @@ table {
>>
>>> +table.shortlog td:first-child{
>>> +     text-align: right;
>>> +}

Show 6 quoted lines

>> Second, I'd rather avoid more advanced CSS constructs; not all web
>> browsers support ':first-child' selector.  On the other hand adding
>> class attribute to handle this would make page slightly larger.
> 
> IIRC :first-child is supported from IE7 onwards. There are hacks to
> make it work on IE6, but I think they are definitely not worth it.

I was thinking here about more exotic web browsers, like Lynx, ELinks, w3m (and w3m in Emacs), Konqueror (KHTML). I think that both Opera and Safari (and other browsers based on WebKit engine) support ':first-child' pseudo-class selector. Also I'd rather not start trend to use more advanced parts of CSS...

On the other hand if we introduce 'age coloring' (as used in projects list view), by using classes age0..age2, then we would be able to use class selector instead of :first-child pseudo-class selector for that.

Show 16 quoted lines

>> Last, and most important: I don't agree with this change.  In my
>> opinion it does not improve layout (and you didn't provide support
>> for this change).  Right-align justification should be sparingly,
>> as it is not natural in left-to-right languages.
> 
> Of course, in my opinion it does improve layout.
> 
> The effect is to right-align the first column of shortlog view, i.e.
> the one holding the date. For dates that are presented as yyyy-mm-dd
> it makes not difference, but when the phrasing is 'X days ago' it
> provides the benefit of aligning the 'days ago' part instead of having
> it ragged. See it live at
> 
> http://git.oblomov.eu/git/shortlog
> 
> and judge for yourselves.

First, it would be nice to have

  See it live at http://git.oblomov.eu/git/shortlog

in the patch comment (between "---\n" line and diffstat).

And I took a look how it looks like, with:
 $ <mark whole thread>
 $ <save as>
 $ git am -3 <file>
 $ stg uncommit -n <n>
 $ stg pop -a
 $ stg push
 $ gitweb-update.sh
 $ <view http://localhost/cgi-bin/gitweb/gitweb.cgi>
 $ ...

Second, even disregarding using ':first-child' pseudo-class selector (it is not that important issue that it is required to have this supported in all browsers), there is problem that this change is _incomplete_. Take a look at 'summary' view (probably most used project specific action): in 'shortlog' you have date right-aligned, while date column in 'heads' and 'tags' parts below you have date left-aligned.

Third, in my opinion it does not improve layout. You align on least important part of relative date specification: on the word "ago". Unit specifiers in relative date specification are of different length so you don't have align (or rather have align in sort subsequences).

Compare:

  15 min ago
  6 hours ago
  10 hours ago
  2 days ago
  2 weeks ago
  6 months ago
  2009-06-12

with

    15 min ago
   6 hours ago
  10 hours ago
    2 days ago
   2 weeks ago
  6 months ago
    2009-06-12

What you probably want to have (and which I am not sure if it is worth complication) is to align on first space/whitespace (align="char" char=" ")

  15 min ago
   6 hours ago
  10 hours ago
   2 days ago
   2 weeks ago
   6 months ago
  2009-06-12

or even

  15 min    ago
   6 hours  ago
  10 hours  ago
   2 days   ago
   2 weeks  ago
   6 months ago
  2009-06-12

-- 
Jakub Narebski
Poland