-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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 meridiem time for zh_cn locale #486
Conversation
Under no circumstances should `am/pm` be shown on `zh_cn`(Chinese) interfaces, so `meridiemUppercase` and `meridiemLowercase` should be translated into `['上午', '下午']` too. There's no difference of upper/lower case in Chinese. Previous behavior: `hh:mm a` => `12:30 am` `hh:mm A` => `12:30 AM` `hh:mm aa` => `12:30 上午` Corrected behavoir: `hh:mm a` => `12:30 上午` `hh:mm A` => `12:30 上午` `hh:mm aa` => `12:30 上午`
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.
@fnlctrl please fix this and also tests: https://github.com/date-fns/date-fns/blob/master/src/locale/zh_cn/build_format_locale/test.js#L207-L241
var meridiemUppercase = ['AM', 'PM'] | ||
var meridiemLowercase = ['am', 'pm'] | ||
var meridiemUppercase = ['上午', '下午'] | ||
var meridiemLowercase = ['上午', '下午'] | ||
var meridiemFull = ['上午', '下午'] |
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 think it should be single variable, like var meridiem = ['上午', '下午']
. And later:
// AM, PM
'A': function (date) {
return (date.getUTCHours() / 12) >= 1 ? meridiemUppercase[1] : meridiemUppercase[0]
}
}
formatters.a = formatters.A
formatters.aa = formatters.A
@@ -38,12 +36,12 @@ function buildFormatLocale () { | |||
|
|||
// AM, PM | |||
'A': function (date) { |
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.
Just omit these tow functions and add formatters.A = formatters.a = formatters.aa
after the declaration of formatters
.
And there are 8 relative failures in the test. You should also modify the test spec so that all tests pass.
Sorry for the delay and it's all fixed now 😄 |
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.
LGTM 👍
@fnlctrl thank you for your contribution! ⭐️ Your fix were released as v1.28.5 (diff: v1.28.4...v1.28.5) |
Under no circumstances should
am/pm
be shown onzh_cn
(Chinese) interfaces, someridiemUppercase
andmeridiemLowercase
should be translated into['上午', '下午']
too. There's no difference of upper/lower case in Chinese.Previous behavior:
hh:mm a
=>12:30 am
hh:mm A
=>12:30 AM
hh:mm aa
=>12:30 上午
Corrected behavoir:
hh:mm a
=>12:30 上午
hh:mm A
=>12:30 上午
hh:mm aa
=>12:30 上午