-
Notifications
You must be signed in to change notification settings - Fork 799
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
WordPress.com Features: Calypso Locale Sync from wp-admin to Calypso #37316
Conversation
Are you an Automattician? Please test your changes on all WordPress.com environments to help mitigate accidental explosions.
Interested in more tips and information?
|
Thank you for your PR! When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:
This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖 The e2e test report can be found here. Please note that it can take a few minutes after the e2e tests checks are complete for the report to be available. Once your PR is ready for review, check one last time that all required checks appearing at the bottom of this PR are passing or skipped. Mu Wpcom plugin:
If you have any questions about the release process, please ask in the #jetpack-releases channel on Slack. |
237d1a0
to
26a0bce
Compare
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.
Working great!
projects/packages/jetpack-mu-wpcom/src/features/calypso-locale-sync/calypso-locale-sync.php
Outdated
Show resolved
Hide resolved
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.
Working great!
projects/packages/jetpack-mu-wpcom/src/features/calypso-locale-sync/calypso-locale-sync.php
Outdated
Show resolved
Hide resolved
projects/packages/jetpack-mu-wpcom/src/features/calypso-locale-sync/calypso-locale-sync.php
Outdated
Show resolved
Hide resolved
a3e600a
to
100a220
Compare
@miksansegundo @DustyReagan I pushed 100a220 which does two things:
What do you think? Feel free to modify! |
734a5f3
to
067a906
Compare
I have fixed the following edge case in 4f93519 When the "Site Default" option is selected in Reproduction steps:
|
I think this PR can mess the translations to users with multiple sites. Steps to reproduce:
|
To fix the behavior I noted above, I think we need to change Calypso so when the user is on a page with nav unification (e.g. My Home, Plans, Domains, etc) and the admin interface is classic, we use the wp-admin locale instead of the Calypso locale. If we do that, I think we can also fix the problem noted by @miksansegundo earlier. |
Thanks for testing!
I just pushed a fix in 4f93519 |
I think the issue I described is still present. It's not an issue about using the site default language, but an issue about having multiple sites with different languages. |
Clarified in p1715689279091669-slack-C06DN6QQVAQ. #37352 will also sync the locale back from Calypso to all classic sites and Site B will have the same locale as Site A. I have some concerns though about users being unaware that the locale setting in wp-admin is actually a global UI language across all sites and WordPress.com, but I guess we can improve that in a follow-up with an informative notice or something else. |
Pretty close! Overall this works quite well, but the Calypso locale doesn't change when I follow these steps:
EDIT: Fixed it in 40b656d |
Related #37352 #19525 pfsHM7-Va-p2
Fixes https://github.com/Automattic/dotcom-forge/issues/6812
Proposed changes:
Calypso Locale Sync from wp-admin to Calypso:
profile_update
to get the wp-admin user profile locale updateen_US
, it's formatted as a language codeen-us
, and when that code doesn't exist, the language variation is removeden
.WPLANG
/wpcom/v2/me/locale
to update Calypso locale (user metalocale
in wpcom database)/wp-admin/options-general.php
and "Site Default" option is selected as the user locale, the hookupdate_option_WPLANG
triggers the Calypso locale sync to the new Site language.Other information:
Jetpack product discussion
Does this pull request change what data or activity we track or use?
Testing instructions:
Test with the API from D148231-code
Go to
/wp-admin/profile.php
on your Atomic test site, change and save the language.You should see the language change reflected on https://wordpress.com/sites
Repeat the test, but this time choose the option "Site Default", which will use the Site language selected in
/wp-admin/options-general.php
Leave selected the option "Site Default" and change the "Site language" from
/wp-admin/options-general.php
to verify the Calypso locale is updated to the new Site language.