Opened 7 years ago
Closed 7 years ago
#42394 closed defect (bug) (fixed)
Customize: Order of fields in date/time control does not honor locale
Reported by: | munyagu | Owned by: | westonruter |
---|---|---|---|
Milestone: | 4.9 | Priority: | normal |
Severity: | normal | Version: | 4.9 |
Component: | Customize | Keywords: | has-patch commit dev-reviewed |
Focuses: | Cc: |
Description
The field order of the schedule setting field in customizer is not changed for each language.
The order is changed in 'Published on' in Edit Post etc.
Attachments (17)
Change History (56)
#2
@
7 years ago
- Component changed from Customize to I18N
- Keywords needs-patch added
- Summary changed from The field order of the schedule setting field in customizer is not changed. to i18n: The field order of the schedule setting field in customizer is not changed.
#3
@
7 years ago
- Component changed from I18N to Customize
- Milestone changed from Awaiting Review to 4.9
- Summary changed from i18n: The field order of the schedule setting field in customizer is not changed. to The field order of the schedule setting field in customizer is not changed.
#4
@
7 years ago
- Summary changed from The field order of the schedule setting field in customizer is not changed. to Customize: Order of fields in date/time control does not honor locale
#5
@
7 years ago
Edit Post screen uses touch_time(), which allows translators to customize the date/time format as needed:
/* translators: 1: month, 2: day, 3: year, 4: hour, 5: minute */ printf( __( '%1$s %2$s, %3$s @ %4$s:%5$s' ), $month, $day, $year, $hour, $minute );
Ideally, the same string should be used in WP_Customize_Date_Time_Control
for consistency, but it uses a completely different markup and a different approach to 12/24 hour format, so the fix appears to be less straightforward than I thought.
#6
@
7 years ago
At the very least we could just detect the ordering of placeholders 1, 2, and 3 and make sure that the corresponding fields for month, year, and day are printed in the same order. This would also need to account for punctuation usage before/after the placeholders.
#7
@
7 years ago
- Keywords has-patch added; needs-patch removed
See 42394.0.diff for one possible solution. It's not perfect in particular because it isn't able to know whether the content between the date and the time is associated with the date or the time.
I think what we need is new translation strings for the date and time separately.
So in addition to:
<?php /* translators: 1: month, 2: day, 3: year, 4: hour, 5: minute */ $datetime_format = __( '%1$s %2$s, %3$s @ %4$s:%5$s' );
We also need:
<?php /* translators: 1: month, 2: day, 3: year, 4: hour, 5: minute */ $date_format = __( '%1$s %2$s, %3$s' ); /* translators: 1: hour, 2: minute, 3: meridian (optional) */ $time_format = __( '%1$s:%2$s %3$s' );
The meridian part gets tricky, however. Right now the meridian part is conditionally included based on whether or not it appears in the time_format
option, and not whether or not it is in the translation string.
Maybe for 4.9 we should fix the issue specifically with the year/month/day fields, but then continue with the current hard-coded approach for the hour/minute/meridian fields, since the time format is much less likely to vary between locales (AFAIK).
#8
@
7 years ago
Or, instead of using the '%1$s %2$s, %3$s @ %4$s:%5$s'
translation string we could instead use the date_format
and time_format
strings to decide the ordering of the fields.
#9
@
7 years ago
Use of the date_format and time_format instead of translation string sounds good to me. At least in Japanese, the order of date is always the same (Y/m/d), but there are some of the formats. E.g. some prefer 2016年11月1日
, but some prefer 2016/11/1
or 2016-11-1
.
Should be their own choices rather than the locale.
#10
@
7 years ago
@Mirucon @SergeyBiryukov Thoughts on 42394.1.diff?
#12
@
7 years ago
- Keywords needs-patch added; has-patch removed
@westonruter Order of the date is correct, but since that is just considered to have Y, y, o, F, m, M, n, j, d
, if the format contains anything else of these string, the extra items will show up after the forms. The screenshot shows what happens with the default Japanese date format - Y年n月j日
#14
@
7 years ago
- Keywords has-patch needs-testing added; needs-patch removed
@Mirucon please clear your cache and try again. jp-date-format.png is what I see if I supply a date format of Y年n月j日
and it looks good to me.
This ticket was mentioned in Slack in #core by jeffpaul. View the logs.
7 years ago
#17
@
7 years ago
@westonruter @Mirucon
I confirmed the patch works successfully.
For the first time, I got the same error with @Mirucon, but minify CSS (customize-controls.min.css
) had not been updated yet. After minify of it and clearing cache, it shows the date with correct format.
#18
@
7 years ago
fyi.
Already Japanese Translation Team is discussing about the fix of duplicate '月'(=month) by modifying Japanese resources. After this fix was merged into the Core, they will work.
#19
@
7 years ago
- Keywords dev-feedback added; needs-testing reporter-feedback removed
@Mirucon in the future you can avoid having to re-minify sources by adding define( 'SCRIPT_DEBUG', true );
to your wp-config.php
This ticket was mentioned in Slack in #core-committers by westonruter. View the logs.
7 years ago
#21
@
7 years ago
Sorry, since that was a new environment, I forgot to add it! Thanks @atachibana, @westonruter
No, it was set - so the cache wasn't cleared properly.
#22
@
7 years ago
In 42394.1.diff, it doesn't look like $format
is used anywhere.
#23
@
7 years ago
@obenland good catch. Removed in 42394.2.diff.
#24
follow-up:
↓ 25
@
7 years ago
This should be applied to posts as well. Shall I create a new ticket or is it handled here?
#25
in reply to:
↑ 24
@
7 years ago
Replying to Mirucon:
This should be applied to posts as well. Shall I create a new ticket or is it handled here?
I don't think that is necessary. Please review how the date is displayed in Gutenberg. It's an actual calendar picker component. That is what core will be moving toward, and the Customizer will likely re-use it instead of what it is including in 4.9, eventually.
#26
@
7 years ago
- Keywords commit dev-reviewed added; dev-feedback removed
42394.2.diff LGTM. Separators can look a little off at times, but I'm not sure this is something we have much control over
#28
@
7 years ago
- Resolution fixed deleted
- Status changed from closed to reopened
If date_format
is set to a custom string without at least one of the month, day, or year tokens, then the Publish Settings are knocked out of whack, as illustrated in the above screenshot with a date_format
of Y-m
.
42394.3.diff includes one possible workaround of falling back to the previously default date format if any of the three placeholders are missing.
#29
@
7 years ago
- Keywords dev-feedback added; dev-reviewed removed
@dlh Good catch, though I wonder how many sites would not include all three parts to the date.
I think, however, that the fallback date format should actually be ISO date format: Y-m-d.
See 42394.4.diff which also ensures that each format character occurs once and only once.
I also realized the foreach
loops could be eliminated.
#32
@
7 years ago
- Keywords has-patch added
@Mirucon I think the point of using ISO as fallback is that it would be an international format and so it wouldn't need to be translated. Otherwise, if it is translatable then it should maybe it should be what I have in comment #7:
<?php /* translators: 1: month, 2: day, 3: year, 4: hour, 5: minute */ $date_format = __( '%1$s %2$s, %3$s' );
#33
@
7 years ago
42394.4.diff works well for me.
I agree that a date_format
without one part of the date would be unusual. The example I was thinking of at the time was a monthly magazine where the day on which a post was published would be unimportant to the site.
That said, if a translated string like __( '%1$s %2$s, %3$s' )
is an option, then is matching the fields to date_format
necessary?
#34
@
7 years ago
@dlh you're right. If __( '%1$s %2$s, %3$s' )
is available then parsing date_format
would not be necessary. I was trying to get by without introducing a new translation string, but if translators are happy with having a locale date format string to translate, we can just use it instead. @Mirucon @atachibana thoughts?
This ticket was mentioned in Slack in #core-i18n by westonruter. View the logs.
7 years ago
#36
@
7 years ago
@westonruter
I believe translators will be happy to do it.
In fact, as @SergeyBiryukov said in comment:5, Edit Post screen has such template string for some time.
This ticket was mentioned in Slack in #core by jeffpaul. View the logs.
7 years ago
#38
@
7 years ago
- Keywords commit dev-reviewed added
We're in RC and with that in string freeze—let's try to avoid introducing new translation strings.
Given how unusual we all think this use case is, I think we should go with 42394.4.diff for now.
Reproduced.
It seems to I18N issue. Customizer schedule's date seems to be fixed as "Month Day, Year".
Step:
Preparation - Set locale to Japanese (日本語)
Open New Post and Click Schedule.
=> You will see correct ordered Year, Month, Day Field boxes.
Open Customizer -> Settings -> Schedule.
=> The Order is Month Day, Year