-
-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
fix: Relative times in Finnish locale #797
Conversation
Codecov Report
@@ Coverage Diff @@
## dev #797 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 156 156
Lines 1111 1118 +7
Branches 205 207 +2
=========================================
+ Hits 1111 1118 +7 Continue to review full report at Codecov.
|
test/locale/fi.test.js
Outdated
|
||
it('Finnish locale relative time in past and future', () => { | ||
expect(dayjs().add(1, 'd').locale('fi').fromNow()) | ||
.toBe('päivän kuluttua') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it will be great if you could add moment.js result in test like:
expect(dayjs().add(1, 'd').locale('fi').fromNow())
.toBe('päivän kuluttua')
expect(dayjs().add(1, 'd').locale('fi').fromNow())
.toBe(moment().add(1, 'd').locale('fi').fromNow())
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Those are actually not equal because the original translation here uses word ”kuluttua” and Moment ”päästä”. They are synonyms.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The word used here is actually better but should they be the same?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, please. Day.js trying to keep everything the same with moment.js at the present.
You can check other locales' test file as a reference.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There seems to be another difference, too. Dayjs tells "2 päivää sitten" and Moment "kaksi päivää sitten". So for some reason, Moment uses words for numbers less than ten. Should this be replicated as well? This seems to be specific for Finnish locale although using integers would be fine.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This difference has already been present here when the original Finnish translation is merged.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I changed these.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cool, thanks
🎉 This PR is included in version 1.8.21 🎉 The release is available on: Your semantic-release bot 📦🚀 |
## [1.8.21](iamkun/dayjs@v1.8.20...v1.8.21) (2020-02-26) ### Bug Fixes * Set + Get accept 'D' as the short version of 'date' ([#795](iamkun/dayjs#795)) ([523c038](iamkun/dayjs@523c038)) * Update DayOfYear plugin type ([#799](iamkun/dayjs#799)) ([5809652](iamkun/dayjs@5809652)) * Update fi (Finnish) locale relativeTime ([#797](iamkun/dayjs#797)) ([4a470fb](iamkun/dayjs@4a470fb))
## [1.8.21](iamkun/dayjs@v1.8.20...v1.8.21) (2020-02-26) ### Bug Fixes * Set + Get accept 'D' as the short version of 'date' ([#795](iamkun/dayjs#795)) ([523c038](iamkun/dayjs@523c038)) * Update DayOfYear plugin type ([#799](iamkun/dayjs#799)) ([5809652](iamkun/dayjs@5809652)) * Update fi (Finnish) locale relativeTime ([#797](iamkun/dayjs#797)) ([4a470fb](iamkun/dayjs@4a470fb))
## [1.8.21](iamkun/dayjs@v1.8.20...v1.8.21) (2020-02-26) ### Bug Fixes * Set + Get accept 'D' as the short version of 'date' ([#795](iamkun/dayjs#795)) ([523c038](iamkun/dayjs@523c038)) * Update DayOfYear plugin type ([#799](iamkun/dayjs#799)) ([5809652](iamkun/dayjs@5809652)) * Update fi (Finnish) locale relativeTime ([#797](iamkun/dayjs#797)) ([4a470fb](iamkun/dayjs@4a470fb))
This fixes problems in Finnish translation (#351) which are now possible to solve after merging #767. Also, the output is now the same as in Moment.js.