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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, 26 June 2009, Giuseppe Bilotta wrote:
> 2009/6/26 Jakub Narebski <jnareb@xxxxxxxxx>:
>> 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;
>>> +}

>> 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.

>> 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]