Skip to content
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

feat: add strict and custom thresholds support for relativeTime plugin #851

Merged
merged 4 commits into from
Mar 27, 2020
Merged

feat: add strict and custom thresholds support for relativeTime plugin #851

merged 4 commits into from
Mar 27, 2020

Conversation

JounQin
Copy link
Contributor

@JounQin JounQin commented Mar 26, 2020

close #830

@codecov
Copy link

codecov bot commented Mar 26, 2020

Codecov Report

Merging #851 into dev will not change coverage by %.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##               dev      #851   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          158       158           
  Lines         1194      1195    +1     
  Branches       243       246    +3     
=========================================
+ Hits          1194      1195    +1     
Impacted Files Coverage Δ
src/plugin/relativeTime/index.js 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 29a7e74...e2fde47. Read the comment docs.

src/utils.js Outdated Show resolved Hide resolved
@iamkun
Copy link
Owner

iamkun commented Mar 27, 2020

After carefully thinking about this PR, it seems it is enough for us to just add a thresholds option. And leave the rest to our end user.

How do you think?

@JounQin
Copy link
Contributor Author

JounQin commented Mar 27, 2020

I think a strict preset would be helpful commonly, actually I don't understand why moment choose the loose way by default which is counterintuitive to me personally.

@iamkun
Copy link
Owner

iamkun commented Mar 27, 2020

Here's my concern, If we added strict mode, should we update all other locales relativeTime part as well? Otherwise, this might be meanless for some locale.

@iamkun
Copy link
Owner

iamkun commented Mar 27, 2020

However, if we just add a custom threshold, and leave the configuration to our user, it will be much better. All our user have to do is to use UpdateLocale to update relativeTime locale, to keep the sync with their custom thresholds.

@JounQin
Copy link
Contributor Author

JounQin commented Mar 27, 2020

Otherwise, this might be meanless for some locale.

Yes, users have to update locales except built-in English.
But using updateLocale seems always required for other locales.
With strict option, we keep built-in locale simple.

Another idea would be supporting strict relativeTime option in locale file, but maybe it's not worth to do it.

module.exports = {
  relativeTime: {
    s: 'a few seconds',
    _s: '%d second', // strict text
    ss: '%d seconds' // fallback to `ss` if `_ss` is not present in `strict` mode
  }
}

@iamkun
Copy link
Owner

iamkun commented Mar 27, 2020

As for now, how about adding the thresholds option in this coming release only. And waiting for issues reply to decide whether to introduce strict mode or not.

@JounQin
Copy link
Contributor Author

JounQin commented Mar 27, 2020

As for now, how about adding the thresholds option in this coming release only. And waiting for issues reply to decide whether to introduce strict mode or not.

I don't think it is feasible about "waiting for issues reply" personally.
There is only a few users care about it, maybe. 😂

@iamkun
Copy link
Owner

iamkun commented Mar 27, 2020

https://momentjs.com/docs/#/customization/relative-time-threshold/
https://momentjs.com/docs/#/customization/relative-time-rounding/

moment has these two options. We can do this too to suit your needs.

{
threshold: {},
rounding: Math.floor
}

@JounQin
Copy link
Contributor Author

JounQin commented Mar 27, 2020

@iamkun Updated according to your last comment.

@iamkun
Copy link
Owner

iamkun commented Mar 27, 2020

That's great! Thanks.

@iamkun iamkun merged commit bd24034 into iamkun:dev Mar 27, 2020
@iamkun
Copy link
Owner

iamkun commented Mar 27, 2020

THX

@iamkun iamkun mentioned this pull request Mar 27, 2020
iamkun pushed a commit that referenced this pull request Apr 10, 2020
## [1.8.24](v1.8.23...v1.8.24) (2020-04-10)

### Bug Fixes

* Add config option to RelativeTime plugin ([#851](#851)) ([bd24034](bd24034))
* add Duration plugin ([#858](#858)) ([d568273](d568273))
* Add en-in, en-tt locales ([#855](#855)) ([c39fb96](c39fb96))
* add isToday, isTomorrow, isYesterday plugins ([#857](#857)) ([fc08ab6](fc08ab6))
* Add option callback to Calendar plugin ([#839](#839)) ([b25be90](b25be90))
* Fix monthsShort for locale fr ([#862](#862)) ([d2de9a0](d2de9a0))
* Update Breton locale (br) meridiem config ([#856](#856)) ([a2a6672](a2a6672))
* Update Ukrainian (uk) locale relative time ([#842](#842)) ([578bc1a](578bc1a))
@iamkun
Copy link
Owner

iamkun commented Apr 10, 2020

🎉 This PR is included in version 1.8.24 🎉

The release is available on:

Your semantic-release bot 📦🚀

andrewhood125ruhuc added a commit to andrewhood125ruhuc/SidRH2 that referenced this pull request May 10, 2022
## [1.8.24](iamkun/dayjs@v1.8.23...v1.8.24) (2020-04-10)

### Bug Fixes

* Add config option to RelativeTime plugin ([#851](iamkun/dayjs#851)) ([bd24034](iamkun/dayjs@bd24034))
* add Duration plugin ([#858](iamkun/dayjs#858)) ([d568273](iamkun/dayjs@d568273))
* Add en-in, en-tt locales ([#855](iamkun/dayjs#855)) ([c39fb96](iamkun/dayjs@c39fb96))
* add isToday, isTomorrow, isYesterday plugins ([#857](iamkun/dayjs#857)) ([fc08ab6](iamkun/dayjs@fc08ab6))
* Add option callback to Calendar plugin ([#839](iamkun/dayjs#839)) ([b25be90](iamkun/dayjs@b25be90))
* Fix monthsShort for locale fr ([#862](iamkun/dayjs#862)) ([d2de9a0](iamkun/dayjs@d2de9a0))
* Update Breton locale (br) meridiem config ([#856](iamkun/dayjs#856)) ([a2a6672](iamkun/dayjs@a2a6672))
* Update Ukrainian (uk) locale relative time ([#842](iamkun/dayjs#842)) ([578bc1a](iamkun/dayjs@578bc1a))
andrewhood125ruhuc added a commit to andrewhood125ruhuc/SidRH2 that referenced this pull request May 10, 2022
## [1.8.24](iamkun/dayjs@v1.8.23...v1.8.24) (2020-04-10)

### Bug Fixes

* Add config option to RelativeTime plugin ([#851](iamkun/dayjs#851)) ([bd24034](iamkun/dayjs@bd24034))
* add Duration plugin ([#858](iamkun/dayjs#858)) ([d568273](iamkun/dayjs@d568273))
* Add en-in, en-tt locales ([#855](iamkun/dayjs#855)) ([c39fb96](iamkun/dayjs@c39fb96))
* add isToday, isTomorrow, isYesterday plugins ([#857](iamkun/dayjs#857)) ([fc08ab6](iamkun/dayjs@fc08ab6))
* Add option callback to Calendar plugin ([#839](iamkun/dayjs#839)) ([b25be90](iamkun/dayjs@b25be90))
* Fix monthsShort for locale fr ([#862](iamkun/dayjs#862)) ([d2de9a0](iamkun/dayjs@d2de9a0))
* Update Breton locale (br) meridiem config ([#856](iamkun/dayjs#856)) ([a2a6672](iamkun/dayjs@a2a6672))
* Update Ukrainian (uk) locale relative time ([#842](iamkun/dayjs#842)) ([578bc1a](iamkun/dayjs@578bc1a))
splashwizard pushed a commit to splashwizard/tracking-time that referenced this pull request Oct 21, 2024
## [1.8.24](iamkun/dayjs@v1.8.23...v1.8.24) (2020-04-10)

### Bug Fixes

* Add config option to RelativeTime plugin ([#851](iamkun/dayjs#851)) ([bd24034](iamkun/dayjs@bd24034))
* add Duration plugin ([#858](iamkun/dayjs#858)) ([d568273](iamkun/dayjs@d568273))
* Add en-in, en-tt locales ([#855](iamkun/dayjs#855)) ([c39fb96](iamkun/dayjs@c39fb96))
* add isToday, isTomorrow, isYesterday plugins ([#857](iamkun/dayjs#857)) ([fc08ab6](iamkun/dayjs@fc08ab6))
* Add option callback to Calendar plugin ([#839](iamkun/dayjs#839)) ([b25be90](iamkun/dayjs@b25be90))
* Fix monthsShort for locale fr ([#862](iamkun/dayjs#862)) ([d2de9a0](iamkun/dayjs@d2de9a0))
* Update Breton locale (br) meridiem config ([#856](iamkun/dayjs#856)) ([a2a6672](iamkun/dayjs@a2a6672))
* Update Ukrainian (uk) locale relative time ([#842](iamkun/dayjs#842)) ([578bc1a](iamkun/dayjs@578bc1a))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

How to get precise second(s) in relativeTime plugin?
2 participants