aurora-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "David McLaughlin" <da...@dmclaughlin.com>
Subject Re: Review Request 26956: Use toLocalTime angular filter rather than duplicate logic in controller
Date Tue, 21 Oct 2014 00:02:57 GMT


> On Oct. 20, 2014, 11:11 p.m., Joshua Cohen wrote:
> > It's not entirely clear from the screenshot that "LOCAL" is a reference to the timezone.
Maybe it's the fact that it's capitalized, but it feels more related to the task state than
the time (I'm envisioning questions like "what does LOCAL - PENDING mean?"). Am I being crazy?
Would it make sense to change it to something like "(local time)", or even better, instead
of hardcoding "LOCAL" maybe just display their actual time zone identifier (or, since that's
a minor hassle from JS, maybe just the offset)?
> 
> David McLaughlin wrote:
>     Hmmm, I can see your point but I found the underline/highlight on the status made
it very clear they were separate entities. Is it too subtle? We also have a request to show
UTC so it might be time to rethink the display of task events. 
>     
>     (FWIW I would much rather show timezone, but it is deprecated in moment.js for the
usual reasons and I don't think offset is a good user experience.)
> 
> Joshua Cohen wrote:
>     Is http://momentjs.com/timezone/ what's deprecated? Or are we just wary of introducing
another dependency?
> 
> David McLaughlin wrote:
>     That is a library to replace the deprecation I was talking about (you can use 'z'
in the formatting string, but it hard fails in certain time zones). But yeah I'd prefer to
avoid adding a new dependency to solve this, at least until we do a better job with JS dependency
management.
> 
> David McLaughlin wrote:
>     Alternative proposal: wrap toLocalTime in a try catch and only show local if the
moment.js call using 'z' fails?
> 
> Joshua Cohen wrote:
>     Sounds reasonable.
>     
>     Is there a ticket tracking the larger problems w/ JS depdency management?

There is sort of a catch-all of a bunch of issues here: https://issues.apache.org/jira/browse/AURORA-451


- David


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26956/#review57459
-----------------------------------------------------------


On Oct. 20, 2014, 10:55 p.m., David McLaughlin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26956/
> -----------------------------------------------------------
> 
> (Updated Oct. 20, 2014, 10:55 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Kevin Sweeney.
> 
> 
> Bugs: AURORA-873
>     https://issues.apache.org/jira/browse/AURORA-873
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Fixes the AM/PM issue by using 24 hour clock format that was already defined in filters.js.

> 
> 
> Diffs
> -----
> 
>   src/main/resources/scheduler/assets/js/controllers.js f8e2eb7b896843bb19ad3ea5108532209603c73c

>   src/main/resources/scheduler/assets/taskStatus.html ae32866ff241e18d04a82e8a7f909cb692657188

> 
> Diff: https://reviews.apache.org/r/26956/diff/
> 
> 
> Testing
> -------
> 
> See attached screenshot. 
> 
> 
> File Attachments
> ----------------
> 
> screenshot
>   https://reviews.apache.org/media/uploaded/files/2014/10/20/d89be4fc-7fae-4746-841a-ccaf66e2439e__Screen_Shot_2014-10-20_at_3.55.15_PM.png
> 
> 
> Thanks,
> 
> David McLaughlin
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message