Skip to content
This repository has been archived by the owner on Apr 12, 2024. It is now read-only.

feat(filter): support conversion to timezone other than UTC #10858

Closed
wants to merge 1 commit into from
Closed

feat(filter): support conversion to timezone other than UTC #10858

wants to merge 1 commit into from

Conversation

shahata
Copy link
Contributor

@shahata shahata commented Jan 24, 2015

No description provided.

@shahata
Copy link
Contributor Author

shahata commented Jan 24, 2015

If this is something we want, I can do it also for ng-model-options, but let's get this in first

expect(date(new Date(Date.UTC(2003, 8, 10, 3, 2, 4)), 'yyyy-MM-dd HH-mm-ssZ', 'GMT+0500')).toEqual('2003-09-10 08-02-04+0500');
});

it('should fallback to default timezone in case an unknown timezone was passed', function() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question: Fallback to local or to UTC?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or throw?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess the current behaviour (before this PR) is to fallback to local time.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I currently fallback to local tz, but I can change it if you think UTC is preferred.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmmm, yes, throwing is also a viable option. we could also have the filter output something that indicates a problem. not sure. I don't think we have any other filters that can fail, I'll look into it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's go with what you have - it is a reasonable default and it is what happens now.

@petebacondarwin
Copy link
Member

LGTM - let's merge and then look at the date input controls

@shahata shahata closed this in c6d8512 Feb 2, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants