Re: [PATCH 1/1] gitweb: javascript ability to adjust time based on timezone

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

 



On Wed, Mar 23, 2011 at 5:08 PM, John 'Warthog9' Hawley
<warthog9@xxxxxxxxxxxxxx> wrote:
> This patch takes the same basic goal, display the appropriate times
> in a given timezone, and implements it in Javascript. ÂThis requires
> adding / using a new class, dtcommit, which is based on the
> dtstart/dtend microformats. ÂAppropriate commit dates are wrapped in
> a span with this class, and a title of the time in ISO8601 format.

John,

Thanks for coding this up.  I tested it on a couple of different
browsers and wanted to share my observations with you.

First, the easy stuff:

1) "git am" complains about whitespace violations

2) HH:MM:SS times need zero padding; otherwise you see:

Tue, 8 Mar 2011 20:29:9 -0700

3) Some of the Javascript functions are double-indented, others single-indented.

4) IE6 does not seem to like ISO 8601 format:

x = new Date("2011-03-09T03:29:09Z");

This sets all fields to NaN.  I suspect that getTime() values
(milliseconds since 1970-01-01) are more portable.

I have attached a trivial patch for these four items; it applies on
top of your original submission.


Some other things that popped up:

5) Some timezone offsets are not a whole number of hours.  Bangalore
time is GMT +0530, for instance.

6) Most U.S. timezones honor daylight savings, so they could be
something like -0700 for part of the year, and -0800 for the rest of
the year.  Picking the "local" option would automatically adjust for
this, but DST limits the usefulness of permanently storing a fixed TZ
offset in the cookie.

7) Looking at a pre-DST commit after DST (or vice versa) can be a
little confusing:

Tue, 8 Mar 2011 20:29:09 -0700 + (19:29 -0800)

I'm not sure which time to believe.  (Although it's likely that a few
weeks after a commit, the exact hour doesn't matter.)

8) The " + " popup menu is a little quirky.  On FF 3.6 it partially
collapses after selecting a value from the dropdown.  On IE6 it shows
"Error in parsing value for 'display'" and does not render.  On Opera
11 it seemed to work OK.

Firefox breakage: http://img217.imageshack.us/f/firefoxa.png/

I'm wondering if there might be a better place on the page to put the
TZ selection.  It isn't immediately obvious to the user what the extra
" + " does, and it seems to cause some issues.

If you decide to keep it where it is, you might want to consider
absolute or fixed positioning so that other elements do not wrap
around it.  IOW it would work more like the dropdown menus on many
sites.

The timezone fixup javascript seemed to work reasonably well, except
for the hiccup with IE6.  Maybe it would be worth splitting this into
two patches: one to rewrite the timestamps, and a second one to add
the TZ selection interface.

Attachment: 0001-gitweb-minor-fixups-to-javascript-localtime-feature.patch
Description: Binary data


[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]