(feat) referenceTime and formats for am calendar

[RomaDotDev]

Backported referenceTime and formats optional params support

[arimus]

Any particular reason this pull request hasn't been accepted?

I did exactly this same work yesterday and was going to submit a pull request, then saw this open one (oops). It's straight forward and would be a very useful addition.

[RomaDotDev]

Thanks for reminding me about this PR, @arimus

Probably I had to promote it more actively =)

[urish]

Please see my next comment:
#78 (comment)

[urish]

@irsick can you please update the PR to the latest source code? I would love it to be a part of the upcoming 1.1.0 release. Thanks!

[RomaDotDev]

Sure, no problem

[urish]

Awesome, much appreciated!

[RomaDotDev]

Done

[urish]

Thank you! What is the reasoning behind changing the order of the arguments?
In general, I think it is better to stay consistent with moment.js

[RomaDotDev]

The reason is pure practical. My team and I work a lot with dates and data conversions in our big Angular 2 project. The situation when you need to change referenceTime happens hardly ever (only once in my practice), while output formats needs to be customized almost every time when calendar pipe is used.

I found out that having
{{myDate | amCalendar:{sameDay:'[Same Day at] h:mm A'} }} is much more natural and intuitive than
{{myDate | amCalendar:null:{sameDay:'[Same Day at] h:mm A'} }}

So you don't need to know the meaning of referenceTime just to customize output formats.

[urish]

Thanks for the explanation @irsick!

@maggiepint Do you know the reasoning behind the order of calendar() method arguments in moment.js?
i.e. why referenceTime comes before format?

[maggiepint]

Talked to Matt about this one, and basically, because we're dumb? I agree that the order you're proposing makes more sense.

I think we could probably do some type checking and make the parameter order work either way, but I'm not eager to change it outright. At this point it's breaking, and a lot of people use it.

[urish]

@maggiepint thank you for your response! You aren't dumb, that's for sure :)
Is making the first argument optional (using typechecking) a feasible option?

I believe we can implement it in angular2-moment, but I would prefer to keep to implementation closely tied to what moment does...

[maggiepint]

Logged in moment/moment#3658

I may try to get to this one for the LOLZ.

[urish]

Awesome @maggiepint! What are the LOLZ? (funny gifs come into mind, but it probably has another meaning in this context)

@irsick would you like to implement a check as maggie suggested to make the parameter order either way, so we stay compatible with moment?

[icambron]

I'd say that the reason Moment has them in a silly order is just history. referenceTime was supported first and then formats was added more recently. The easiest way to do the reverse-compat was to tack the new arguments on at the end. Probably would have been wiser to flip the order then and do type-checking for reverse compat, but we are not wise.

I agree the order is ungood. And having the first object be a Moment causes lots of confusion on its own. Moment's API doesn't use option objects, so it would be weird to add them now, but that would actually be my preferred way of passing in referenceTime. Back in reality, yeah, I agree that Moment should be willing to take either arg solo.

[RomaDotDev]

Updated PR to support both orders of arguments.

Even if I personally agree with object argument proposed by @icambron for the sake of pipe syntax simplicity I made it based on type check by @maggiepint.

[urish]

Thank you @irsick for following up with all the changes, and everyone on this thread for the valuable discussion.

[urish]

Released as part of 1.1.0