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.

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

Add new field to translate in theme.json #292

Merged
merged 1 commit into from
Dec 2, 2021
Merged

Add new field to translate in theme.json #292

merged 1 commit into from
Dec 2, 2021

Conversation

oandregal
Copy link
Contributor

@oandregal oandregal commented Dec 2, 2021

Depends on WordPress/gutenberg#37038

In WordPress/gutenberg#37038 Gutenberg is adding a new field to translate for the font sizes stored within a theme.json. This PR adds it to the i18n command.

Setup

  • See https://make.wordpress.org/cli/handbook/contributions/pull-requests/#setting-up
  • To run the behat tests you also need a working mysql locally:
    • Install the packages if you don't have them. In linux: sudo apt install jq mysql-server mysql-client php-mysql
    • composer install && composer behat => it didn't setup the database for me (mysql 8) so I did;
      • sudo mysql -u root
      • CREATE DATABASE IF NOT EXISTS wp_cli_test;
      • CREATE USER 'wp_cli_test'@'localhost' IDENTIFIED WITH mysql_native_password BY 'password1';
      • GRANT ALL PRIVILEGES ON wp_cli_test.* TO "wp_cli_test"@"localhost"

How to test

  • Check out this PR locally and cd to the directory.
  • composer install
  • Edit ThemeJsonExtractor.php by substituting the line 148 $json = self::remote_get( self::THEME_JSON_SOURCE ); by $json = '';.
  • Create a new directory called foo-theme and create a theme.json file within with the following contents:
{
  "version": "2",
  "settings": {
    "typography": {
      "fontSizes": [
        { "slug": "small", "size": "12px", "name": "Small", "alias": "S" }
      ]
    }
  }
}
  • Run vendor/bin/wp i18n make-pot foo-theme
  • Verify that there's a foo-theme/foo-theme.pot file and that it has the name and alias strings:
#: ...

#: theme.json
msgctxt "Font size name"
msgid "Small"
msgstr ""

#: theme.json
msgctxt "Short font size name"
msgid "S"
msgstr ""

@schlessera schlessera added this to the 2.2.10 milestone Dec 2, 2021
@schlessera schlessera merged commit e74ecbc into wp-cli:master Dec 2, 2021
@schlessera
Copy link
Member

Thanks for the PR, @oandregal !

@oandregal oandregal deleted the add/new-field-to-translate branch December 2, 2021 13:00
@oandregal
Copy link
Contributor Author

ok, that was fast 😅 I was about to push a tweak to the behat tests to include this case. Also, I was waiting until the Gutenberg PR that uses it at WordPress/gutenberg#37038 was merged, to be 100% certain this was a change we needed.

If we're aiming to release 2.5.1 these days, I guess is fine to go ahead with this change. In the worst-case scenario (Gutenberg PR doesn't get merged), we'd need to remove this key in the future, but with no urgency as everything will work as expected.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants