issue #202 ability to provide options to the MomentModule #209
Conversation
add `forRoot` to `MomentModule` to allow passing of `NgxMomentOptions`, can be easily extended to support other options (both global and local to pipes) added optional parameter to constructor of DurationPipe to provide options and allows pipe to specify `relativeTimeThreshold` units added tests to support
src/duration.pipe.ts
Outdated
| transform(value: any, ...args: string[]): string { | ||
| if (typeof args === 'undefined' || args.length !== 1) { | ||
| throw new Error('DurationPipe: missing required time unit argument'); | ||
| } | ||
| return moment.duration(value, args[0] as moment.unitOfTime.DurationConstructor).humanize(); | ||
| } | ||
|
|
||
| private _applyOptions(momentOptions: NgxMomentOptions): void { | ||
| if (!!!momentOptions) { |
There was a problem hiding this comment.
Why triple negation? I'd rather use single negation
There was a problem hiding this comment.
I felt triple negation was safer, equivalent to if (!(!!momentOptions)). If you prefer single negation ill make the change
| beforeEach(() => pipe = new DurationPipe(momentOptions)); | ||
|
|
||
| it(`should convert '50 minutes' to '50 minutes' when relativeTimeThreshold for 'm' unit is set to 59`, () => { | ||
| expect(pipe.transform(50, 'minutes')).toEqual('50 minutes'); |
| * @example by default more than 45 seconds is considered a minute, more than 22 hours is considered a day and so on. | ||
| * So settings the unit 'm' to `59` will adjust the `relativeTimeThreshold` and consider more than 59 minutes | ||
| * to be an hour (default is `45 minutes`) | ||
| */ |
There was a problem hiding this comment.
Thanks for adding this doc comment. Can you please also add a note to the README explaining this?
There was a problem hiding this comment.
README updated in next commit
src/moment-options.ts
Outdated
| @@ -0,0 +1,16 @@ | |||
| import { InjectionToken } from '@angular/core'; | |||
|
|
|||
| export const NGX_MOMENT_OPTIONS: InjectionToken<string> = new InjectionToken<string>('NGX_MOMENT_OPTIONS'); | |||
There was a problem hiding this comment.
According to docs, it seems like the type parameter for InjectionToken should be the interface name (i.e. NgxMomentOptions):
There was a problem hiding this comment.
Yep, my bad and good spot, updated in next commit.
and fix generic type declaration of `NGX_MOMENT_OPTIONS` injection token
|
Thank you! |
|
@urish , also looking for this. Any chance you could cut a new release to include this? |
|
@deeg I plan to, when I get a chance, I'm currently on a long vacation in Japan |
|
@urish , no rush, have fun on the vacation! I hear soon all the cherry blossoms will be in full force. Will you get to enjoy that? |
|
@deeg Hopefully so! You can read about our journey so far in my blog post. Regarding this PR - released as part of 3.4.0 |
Add ability to provide options to the moment module using the
forRootconvention to supply options globally or per pipe