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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Feb 16, 2020
Merged

fix: Relative times in Finnish locale #797

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8ea3b66
fix: Relative times in Finnish locale
ttsirkia Feb 15, 2020
4b9ee07
fix: Use same output in Finnish locale as in Moment.js
ttsirkia Feb 16, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 41 additions & 20 deletions src/locale/fi.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,36 @@
// Finnish [fi]
import dayjs from 'dayjs'

function relativeTimeFormatter(number, withoutSuffix, key, isFuture) {
const past = {
s: 'muutama sekunti',
m: 'minuutti',
mm: '%d minuuttia',
h: 'tunti',
hh: '%d tuntia',
d: 'päivä',
dd: '%d päivää',
M: 'kuukausi',
MM: '%d kuukautta',
y: 'vuosi',
yy: '%d vuotta'
}
const future = {
s: 'muutaman sekunnin',
m: 'minuutin',
mm: '%d minuutin',
h: 'tunnin',
hh: '%d tunnin',
d: 'päivän',
dd: '%d päivän',
M: 'kuukauden',
MM: '%d kuukauden',
y: 'vuoden',
yy: '%d vuoden'
}
return ((isFuture && !withoutSuffix) ? future : past)[key].replace('%d', number)
}

const locale = {
name: 'fi', // Finnish
weekdays: 'sunnuntai_maanantai_tiistai_keskiviikko_torstai_perjantai_lauantai'.split('_'), // Note weekdays are not capitalized in Finnish
Expand All @@ -10,29 +40,20 @@ const locale = {
monthsShort: 'tammi_helmi_maalis_huhti_touko_kesä_heinä_elo_syys_loka_marras_joulu'.split('_'),
ordinal: n => `${n}.`,
weekStart: 1,
/*
* This relativeTime is currently configured for having proper past
* tense forms since Finnish needs a separate version for future tense
* and I think past tense is a more common use case for this kind of
* library.
*
* Doing this properly requires this issue to be fixed:
* https://github.com/iamkun/dayjs/issues/302
*/
relativeTime: {
future: '%s kuluttua',
past: '%s sitten',
s: 'muutama sekunti', // for past tense
m: 'minuutti', // for past tense
mm: '%d minuuttia', // for past tense
h: 'tunti', // for past tense
hh: '%d tuntia', // for past tense
d: 'päivä', // for past tense
dd: '%d päivää', // for past tense
M: 'kuukausi', // for past tense
MM: '%d kuukautta', // for past tense
y: 'vuosi', // for past tense
yy: '%d vuotta' // for past tense
s: relativeTimeFormatter,
m: relativeTimeFormatter,
mm: relativeTimeFormatter,
h: relativeTimeFormatter,
hh: relativeTimeFormatter,
d: relativeTimeFormatter,
dd: relativeTimeFormatter,
M: relativeTimeFormatter,
MM: relativeTimeFormatter,
y: relativeTimeFormatter,
yy: relativeTimeFormatter
},
formats: {
LT: 'HH.mm',
Expand Down
27 changes: 27 additions & 0 deletions test/locale/fi.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
import MockDate from 'mockdate'
import dayjs from '../../src'
import relativeTime from '../../src/plugin/relativeTime'
import '../../src/locale/fi'

dayjs.extend(relativeTime)

beforeEach(() => {
MockDate.set(new Date())
})

afterEach(() => {
MockDate.reset()
})

it('Finnish locale relative time in past and future', () => {
expect(dayjs().add(1, 'd').locale('fi').fromNow())
.toBe('päivän kuluttua')
Copy link
Owner

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())

Copy link
Contributor Author

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.

Copy link
Contributor Author

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?

Copy link
Owner

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.

Copy link
Contributor Author

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.

Copy link
Contributor Author

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I changed these.

expect(dayjs().add(2, 'd').locale('fi').fromNow())
.toBe('2 päivän kuluttua')
expect(dayjs().add(2, 'd').locale('fi').fromNow(true))
.toBe('2 päivää')
expect(dayjs().add(-1, 'd').locale('fi').fromNow())
.toBe('päivä sitten')
expect(dayjs().add(-2, 'd').locale('fi').fromNow())
.toBe('2 päivää sitten')
})