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

process ISO 8601 formatted date/time strings with date filter #125

Closed
wants to merge 4 commits into from

Conversation

groner
Copy link
Contributor

@groner groner commented Nov 5, 2010

This actually just uses the parsing that the Date builtin provides. If it fails to parse the date, behavior does not change.

Tested in FF 3.6 and Chrome 7.0.

@IgorMinar
Copy link
Contributor

Chrome and FF are currently the only browsers that can parse ISO 8601. Safari doesn't parse it, and I'd be surprised if IE did.

Additionally, Date#toISOString is not supported by IE, so we can't have it in TzDate, which should, as a good fake, be a the lowest common denominator of Date implementations. We should use angular.Date.toString in the test instead of modifying the mock.

The implementation with all the if tests and weird branching is a bit hacky. I'll look at this code tomorrow and see if I can come up with a better solution. Of course you are welcome to offer a better implementation.

Lastly, I think I'd rather limit the supported input to ISO 8601 (by using angular.String.toDate) only and not just any string. This should eliminate browser inconsistencies when it comes to parsing.

I'll have a fresh look at this tomorrow, but feel free to discuss or work on this issue further.

@IgorMinar
Copy link
Contributor

I cooked up a fix here: #129

@groner
Copy link
Contributor Author

groner commented Nov 7, 2010

Thanks for the feedback, Igor. I didn't realize there already is a parser for this format. I agree that it doesn't need to handle other formats that Date.parse might.

I'm closing this since your fix seems like the way to go.

groner pushed a commit to groner/angular.js that referenced this pull request Sep 5, 2011
This pull request was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants