wp-cli / i18n-command

Provides internationalization tools for WordPress projects.
MIT License
96 stars 52 forks source link

theme.json: find `title` string within `styles.blocks.variations` #405

Closed oandregal closed 1 month ago

oandregal commented 1 month ago

Related PRs in this repo: https://github.com/wp-cli/i18n-command/pull/254 https://github.com/wp-cli/i18n-command/pull/292 https://github.com/wp-cli/i18n-command/pull/306 Related Gutenberg PR for this string: https://github.com/WordPress/gutenberg/pull/62552

There's a new feature coming for WordPress 6.6 that allows theme authors to register block style variations through theme.json. Block style variations have a label that is displayed to users, so it needs to be translated. I just found out that there is a missing place where the label wasn't picked from. This PR follows suit on what is done in Gutenberg at https://github.com/WordPress/gutenberg/pull/62552 There's an upcoming WordPress core PR as well.

Setup

brew install jq mysql
brew service start mysql
mysql_secure_installation
sudo mysql -u root -p

# MySQL Session
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";
\q

Test

Before running any of the test (manual or automattic), make sure to set the THEME_JSON_SOURCE to empty so the ThemeJsonExtractor picks the the local backup file with the changes at assets/theme-i18n.json.

Automated tests:

composer install
composer behat -- features/makepot.feature

Manual testing:

{
    "styles": {
      "blocks": {
        "variations": {
          "myVariation": {
            "title": "My variation",
            "blockTypes": [ "core/group" ],
            "color": {
              "background": "red"
            }
          }
        }
      }
    }
}
{
    "styles": {
      "blocks": {
        "variations": {
          "myVariation": {
            "title": "Other variation",
            "blockTypes": [ "core/group" ],
            "color": {
              "background": "red"
            }
          }
        }
      }
    }
}
#: styles/ember.json
msgctxt "Style variation name"
msgid "Other variation"
msgstr ""

#: theme.json
msgctxt "Style variation name"
msgid "My variation"
msgstr ""

Comments

This also needs a WordPress core PR. https://github.com/WordPress/gutenberg/pull/62552 will be backported with the necessary changes.

oandregal commented 1 month ago

It's not ready yet, hence the draft status. Will finalize it in the following days.

swissspidy commented 1 month ago

Why is this kind of stuff happening so late in the cycle? 🤔

FWIW we now have an automated GitHub Actions workflow updating the schema in this repo. It's based on the schema in WordPress core trunk.

aaronrobertshaw commented 1 month ago

This is the first I've setup a wp-cli command env locally so have limited context around how this should be done. Please take the following with a grain of salt 🙂

✅ Setting up the env went smoothly ✅ This PR manually tests as advertised following test instructions directly ✅ The generated pot file looks to contain the desired data

Screenshot 2024-06-14 at 11 27 45 AM

❓ The automated tests do fail locally for me as well

Screenshot 2024-06-14 at 11 32 30 AM

Why is this kind of stuff happening so late in the cycle? 🤔

This is probably due to an oversight of mine, sorry.

There was a bug fix required for the new section styles feature so that keys in the theme.json could reliably be presumed to be slugs and prevent an apparent duplication of the style within the UI.

swissspidy commented 1 month ago

The automated tests do fail locally for me as well

It‘s because this change is not in core trunk yet.

oandregal commented 1 month ago

It‘s because this change is not in core trunk yet.

How the behat tests find the schema? Don't they use the THEME_JSON_SOURCE? I've set that constant to empty (so, supposedly, it's using the local file with the updated property) but the test still fails 🤔

Am I running the right command?

composer behat -- features/makepot.feature
swissspidy commented 1 month ago

Yes that's the command. Yes they use THEME_JSON_SOURCE (the remote file), falling back to the local file (THEME_JSON_FALLBACK).

That's why the tests are failing on CI.

Are you saying the tests are failing locally for you when trying to force using the fallback?

oandregal commented 1 month ago

Are you saying the tests are failing locally for you when trying to force using the fallback?

Yeah, exactly 😕 I set that constant to empty and executed the local tests but they still fail:

Captura de ecrã 2024-06-14, às 08 47 26
oandregal commented 1 month ago

Pushed a new test for files within styles/ folder as well 21ffdee

oandregal commented 1 month ago

Related WordPress PR with the schema changes at https://github.com/WordPress/wordpress-develop/pull/6825 I'll merge it shortly, to unblock CI tests here.

oandregal commented 1 month ago

Committed the schema changes at https://core.trac.wordpress.org/changeset/58412 I presume the CI test requires a re-triggering at some point for tests to pass.

ernilambar commented 1 month ago

I have re-run the tests.

swissspidy commented 1 month ago

@oandregal Looking at your core changeset, styles needs to be under settings. In your test it's not under settings. So the test needs to be fixed.

oandregal commented 1 month ago

I've pushed a couple of fixes to the test: c918b7ab629439fb5caf731985bc0fbadafde361 and 8465789f2539fd50cba3933670f21120762af4be. It's now passing for me locally (both setting THEME_JSON_SOURCE to empty to use the local assets and leaving it as it is to use the remote one from core).