Skip to content
This repository has been archived by the owner on Nov 30, 2021. It is now read-only.

fixes amDateFormat unixdatetime bug #14

Merged
merged 3 commits into from
Sep 28, 2013
Merged

fixes amDateFormat unixdatetime bug #14

merged 3 commits into from
Sep 28, 2013

Conversation

AlmogBaku
Copy link
Contributor

No description provided.

@urish
Copy link
Owner

urish commented Sep 27, 2013

Hi Almog,

Can you please explain the issue your commit is trying to fix?

Thanks,
Uri

@AlmogBaku
Copy link
Contributor Author

if I send time(miliseconds) to the filter it not recognize it as time, but
it recognize it as string.
By this commit it will recognize string-number and int-number as well, and
will parse it as expected.

On Sat, Sep 28, 2013 at 1:51 AM, Uri Shaked notifications@github.comwrote:

Hi Almog,

Can you please explain the issue your commit is trying to fix?

Thanks,
Uri


Reply to this email directly or view it on GitHubhttps://github.com//pull/14#issuecomment-25282818
.

@urish
Copy link
Owner

urish commented Sep 27, 2013

And doesn't passing 'X' as an argument to the filter achieve the same?

i.e. {myVar | amDateFormat:'X'}

See http://momentjs.com/docs/#/parsing/string-format/

@AlmogBaku
Copy link
Contributor Author

No.. I mean to translate myVar to Date() object..

If you wish to translate timestamp to Date object, you must send a number
argument(int).. BUT, when I send the argument using angular filter it
send me the number as string...

My commit will recognize if the string is number, and if so it will parse
it to integer then it will parse it to Date object.

On Sat, Sep 28, 2013 at 2:02 AM, Uri Shaked notifications@github.comwrote:

And doesn't passing 'X' as an argument to the filter achieve the same?

i.e. {myVar | amDateFormat:'X'}

See http://momentjs.com/docs/#/parsing/string-format/


Reply to this email directly or view it on GitHubhttps://github.com//pull/14#issuecomment-25283241
.

@urish
Copy link
Owner

urish commented Sep 27, 2013

Interesting, do you have any idea why does it pass it as a string for you?

I just tested the following
{{15 | amDateFormat}} and it seems to pass it correctly (as int) and not as a string...

@AlmogBaku
Copy link
Contributor Author

cool.
In my case- I extract the numbers from object key by ngRepeat..

ng-repeat="(day, dayEvents) in events"

And I set it as integer in the object(from Date object).
Anyway, this checks solve my issue and retrieve me a fine Date object.

Sabbath Shalom!

@urish
Copy link
Owner

urish commented Sep 28, 2013

So in your case day is the object that you want to print with amDateFormat and it's a string because this is hour your dictionary is structured?

@AlmogBaku
Copy link
Contributor Author

This is pretty wired.. but it seems that when I run ngRepeat the key is
always string

On Sat, Sep 28, 2013 at 2:49 PM, Uri Shaked notifications@github.comwrote:

So in your case day is the object that you want to print with
amDateFormat and it's a string because this is hour your dictionary is
structured?


Reply to this email directly or view it on GitHubhttps://github.com//pull/14#issuecomment-25296691
.

@urish
Copy link
Owner

urish commented Sep 28, 2013

I see... Well, the fix seems appropriate then.

Can you please make sure it passed jshint + tests and also create a new test case for your fix? You can easily run the tests + jshint checks by executing grunt test.

The tests sit in tests.js. You can use the existing tests as an example. If you need any assistance with writing the new test case or understanding jshint output, just drop me a word.

Thanks :-)

making it pass the tests
@AlmogBaku
Copy link
Contributor Author

done :)

(btw, cool! thanks to you I now used bower grunt and angular-tests for the first time)

urish added a commit that referenced this pull request Sep 28, 2013
fixes amDateFormat unixdatetime bug
@urish urish merged commit 4f9cd8e into urish:master Sep 28, 2013
@urish
Copy link
Owner

urish commented Sep 28, 2013

Excellent, hope you had a positive experience with them :-)

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.

None yet

2 participants