New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding settings to configure relative time thresholds between time units #1663

Merged
merged 2 commits into from May 22, 2014

Conversation

Projects
None yet
3 participants
@skastel
Contributor

skastel commented May 16, 2014

This new feature allows a user to set custom thresholds for the division between time units for relative time strings.

Example:
The default threshold between hours and days as the unit for relative time is 22 hours. With this feature the user can now set a custom threshold like so:

var m = moment().subtract('hours', 23);
m.fromNow(); // Shows "a day ago"
moment.relativeTimeThreshold('h', 24)
m.fromNow(); // Shows "23 hours ago"
@Menelion

This comment has been minimized.

Menelion commented May 16, 2014

Looks good to me. @ichernev, are those tests enough or should there be tests for months and years, also?

@ichernev

This comment has been minimized.

Contributor

ichernev commented May 21, 2014

@skastel looks good. You can add a few more tests as Oire suggested, but also make them on the edge -- just under the threshold and just over the threshold to make sure its working on both ends :)

@skastel

This comment has been minimized.

Contributor

skastel commented May 22, 2014

@ichernev updated the tests to test on either side of custom and default thresholds now, should be pretty comprehensive.

Menelion pushed a commit that referenced this pull request May 22, 2014

Andre Polykanine A.K.A. Menelion Elensúlë
Merge pull request #1663 from skastel/relative-time-threshold-settings
Adding settings to configure relative time thresholds between time units

@Menelion Menelion merged commit 97afb31 into moment:develop May 22, 2014

1 check passed

continuous-integration/travis-ci The Travis CI build passed
Details
@Menelion

This comment has been minimized.

Menelion commented May 22, 2014

@skastel, thanks a lot! Merging.

@skastel

This comment has been minimized.

Contributor

skastel commented May 22, 2014

Awesome, thanks for merging it in!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment