-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Date: add relative time translations for moment.js #53931
Conversation
This pull request has changed or added PHP files. Please confirm whether these changes need to be synced to WordPress Core, and therefore featured in the next release of WordPress. If so, it is recommended to create a new Trac ticket and submit a pull request to the WordPress Core Github repository soon after this pull request is merged. If you're unsure, you can always ask for help in the #core-editor channel in WordPress Slack. Thank you! ❤️ View changed files❔ lib/compat/wordpress-6.4/script-loader.php ❔ lib/load.php |
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.
Thanks so much for the fix @ramonjd!
✅ Confirmed the missing strings prior to applying this PR
✅ With this PR applied, the settings are identical as without it, except for the additional relative strings (when running JSON.stringify( wp.date.getSettings() );
to compare strings)
✅ Visually, the code here otherwise matches what's in core
LGTM! ✨
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!
My concern is that this code might be better written in lib/compat/wordpress-6.4/script-loader.php
.
I think this hook expects all relative times to translate correctly on all WordPress versions supported by the Gutenberg plugin. And if the minimum WordPress version that the Gutenberg plugin supports has this code, this hook will not be needed.
I think the lib/compat/wordpres-{X.Y}
directory will be removed as the Gutenberg plugin supports more WordPress versions, so you can avoid forgetting to remove this hook.
What do you think?
Yes, it totally makes more sense to store this under a |
…the case of indefinite articles
… as it reflects the intended destination in Core and also communicates compatibility.
2a73135
to
d21cbdd
Compare
Resolves #53888
What?
Overwrites
wp-date
script string to include newrelativeTime
translations.The inline script is generated in Core here: https://github.com/WordPress/wordpress-develop/blob/bf00a673a5f1d7f4707a6f914e1408526a8dc06a/src/wp-includes/script-loader.php#L430 and will need to be synced.
See: https://momentjscom.readthedocs.io/en/latest/moment/07-customization/07-relative-time/
Why?
The settings strings were never synced to Core.
Now that we're using
relativeTime
in humanTimeDiff, we need the translations.Unfortunately,
%d minutes
and others have not yet been translated.How?
Hooking into
wp_default_scripts
and overwriting the settings of the registeredwp-date
script.The other option is to re-add the inline script, which will also work, but it will output 2 x inline scripts, e.g.,
Testing Instructions
Change your editor/site to another language via Settings > General
Since the relevant translations have not been added, you can check that the
wp.date
settings have been updated by heading to/wp-admin/site-editor.php
and, in the browser console, runningwp.date.getSettings()
The
relative
object should match the requirements of https://momentjscom.readthedocs.io/en/latest/moment/07-customization/07-relative-time/