19701 try to come up with a solution where html tags in translations are not reliant on the order by vraja-pro · Pull Request #21938 · Yoast/wordpress-seo

vraja-pro · 2024-12-31T10:07:57Z

Context

We want to catch translation errors from breaking our UI.

Summary

This PR can be summarized in the following changelog entry:

Implements helper function to catch errors in translations.

Relevant technical choices:

Test instructions

Test instructions for the acceptance test before the PR gets merged

This PR can be acceptance tested by following these steps:

Go to the Yoast SEO-> Settings -> Site basics -> Tagline , There should be a comment under the field:

This field updates the tagline in your WordPress settings

Remember that field and change to other language besides English.
Install the plugin Loco translate to edit your translation
Go to WordPress translations search for your language and choose stable version.
Search for the translation: "This field updates the %1$stagline in your WordPress settings%2$s."
Tick the translation and on the bottom of the screen you will have the option to download the translation.
Go to your downloads directory and rename the file by removing the the first part: wp-plugins- and it ends with the underscore and the country locale.
Go back the site, navigate to Loco translate->Yoast SEO, you will have a small icon to upload po file, click on it and select the file you renamed, select to location to "Custom", Make sure the name of the file matches the name as instructed above the upload button.
Then go back to the overview tab and edit the translation you just uploaded.
Search for the string "This field updates the %1$stagline in your WordPress settings%2$s."
Switch the placeholders and save: "This field updates the %2$stagline in your WordPress settings%1$s."
Go back to Yoast SEO-> Settings -> Site basics -> Tagline, look at the line under the field and check the tags are visible and there is no link: "This field updates the tagline in your WordPress settings."
Open dev tool and check the console has an error for that translation that starts with Error in translation for... and mention the translation and the error.

Relevant test scenarios

Changes should be tested with the browser console open
Changes should be tested on different posts/pages/taxonomies/custom post types/custom taxonomies
Changes should be tested on different editors (Default Block/Gutenberg/Classic/Elementor/other)
Changes should be tested on different browsers
Changes should be tested on multisite

Test instructions for QA when the code is in the RC

QA should use the same steps as above.

QA can test this PR by following these steps:

Impact check

This PR affects the following parts of the plugin, which may require extra testing:

UI changes

This PR changes the UI in the plugin. I have added the 'UI change' label to this PR.

Other environments

This PR also affects Shopify. I have added a changelog entry starting with [shopify-seo], added test instructions for Shopify and attached the Shopify label to this PR.

Documentation

I have written documentation for this change. For example, comments in the Relevant technical choices, comments in the code, documentation on Confluence / shared Google Drive / Yoast developer portal, or other.

Quality assurance

I have tested this code to the best of my abilities.
During testing, I had activated all plugins that Yoast SEO provides integrations for.
I have added unit tests to verify the code works as intended.
If any part of the code is behind a feature flag, my test instructions also cover cases where the feature flag is switched off.
I have written this PR in accordance with my team's definition of done.
I have checked that the base branch is correctly set.

Innovation

No innovation project is applicable for this PR.
This PR falls under an innovation project. I have attached the innovation label.
I have added my hours to the WBSO document.

Fixes Try to come up with a solution where HTML tags in translations are not reliant on the order

coveralls · 2024-12-31T13:23:04Z

Pull Request Test Coverage Report for Build 91da0f367dfc4b28a114602b154580fcc70de6b1

Details

0 of 6 (0.0%) changed or added relevant lines in 2 files are covered.
No unchanged relevant lines lost coverage.
Overall first build on 19701-try-to-come-up-with-a-solution-where-html-tags-in-translations-are-not-reliant-on-the-order at 33.37%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
packages/js/src/settings/routes/site-basics.js	0	1	0.0%
packages/js/src/helpers/i18n.js	0	5	0.0%

Totals
Change from base Build 9c66956756c0900e29fb6645e4308dfb47898de1:	33.4%
Covered Lines:	2033
Relevant Lines:	5789

💛 - Coveralls

igorschoester

CR 🏗️

Edit: I think the changelog entries should be user facing. Maybe not the technical one that is there now. But maybe improvement that translations will now fall back to English?

igorschoester · 2025-01-06T11:13:44Z

As you see in the comments you got, the helpers package should not be added to.
The reason for this is that we don't want to maintain a random assortment of helpers in a helper package. If there is a need for a overarching package. You could create a specific one. The smaller and targeted, the less maintenance it needs?

Why did you add it to helpers in the first place?

I was thinking it would be easier to use in the rest of the addon/premium

Easier than what?

I moved it to the helpers folder, but I'm thinking we should have a separate package or fix the issue at the source( fixing createInterpolateElement).

Maybe. But it seems to me this is very WP plugin related. And we have API (editor-modules) to expose to our other plugins.

Also implements it in the site basic page tests for i18nCreateInterpolateElement fix lint restore testURL remove jsdom

This reverts commit e2421b1.

vraja-pro · 2025-04-08T09:48:33Z

Followup comment: we decided to keep it the way it is and write the translation in a way to avoid injecting tags,

vraja-pro · 2025-04-08T09:49:13Z

Temporary solution.

…-with-a-solution-where-html-tags-in-translations-are-not-reliant-on-the-order

refine test

igorschoester

CR && AT ✅

Though I did not manage to get the Loco translation thing working. I just adapted the source string to be reversed already 😉

I'm holding of merging for now, as I'm not sure about our plan for implementation? This is just adding it in one location anyway. Slack question

igorschoester · 2025-04-14T13:53:04Z

Ok, can be merged. More follow-up needed and created this issue to keep track of that

vraja-pro added the changelog: non-user-facing Needs to be included in the 'Non-userfacing' category in the changelog label Dec 31, 2024

This comment was marked as outdated.

Sign in to view

leonidasmi changed the base branch from trunk to feature/plugin-fixes January 2, 2025 09:55

igorschoester suggested changes Jan 6, 2025

View reviewed changes

igorschoester changed the base branch from feature/plugin-fixes to trunk January 7, 2025 10:44

vraja-pro added 3 commits January 14, 2025 09:48

add helper to catch errors for interpolated strings

e2421b1

Also implements it in the site basic page tests for i18nCreateInterpolateElement fix lint restore testURL remove jsdom

Revert "add helper to catch errors for interpolated strings"

3500d3c

This reverts commit e2421b1.

refactor i18nCreateInterpolateElement

e258374

vraja-pro force-pushed the 19701-try-to-come-up-with-a-solution-where-html-tags-in-translations-are-not-reliant-on-the-order branch from 370ff4d to e258374 Compare January 14, 2025 11:34

rename

c99aad0

vraja-pro closed this Jan 22, 2025

vraja-pro reopened this Apr 8, 2025

vraja-pro added 2 commits April 8, 2025 12:50

Merge remote-tracking branch 'origin/trunk' into 19701-try-to-come-up…

b969489

…-with-a-solution-where-html-tags-in-translations-are-not-reliant-on-the-order

tests: safeCreateInterpolateElement

ee859f8

refine test

vraja-pro force-pushed the 19701-try-to-come-up-with-a-solution-where-html-tags-in-translations-are-not-reliant-on-the-order branch from 29158da to ee859f8 Compare April 8, 2025 11:44

Remove mock and use actual implementation

32a765b

igorschoester approved these changes Apr 14, 2025

View reviewed changes

igorschoester added this to the 25.0 milestone Apr 14, 2025

igorschoester merged commit 436e0fd into trunk Apr 14, 2025
18 checks passed

igorschoester deleted the 19701-try-to-come-up-with-a-solution-where-html-tags-in-translations-are-not-reliant-on-the-order branch April 14, 2025 13:53

Conversation

vraja-pro commented Dec 31, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Context

Summary

Relevant technical choices:

Test instructions

Test instructions for the acceptance test before the PR gets merged

Relevant test scenarios

Test instructions for QA when the code is in the RC

Impact check

UI changes

Other environments

Documentation

Quality assurance

Innovation

Uh oh!

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

coveralls commented Dec 31, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request Test Coverage Report for Build 91da0f367dfc4b28a114602b154580fcc70de6b1

Details

💛 - Coveralls

Uh oh!

igorschoester left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

igorschoester Jan 6, 2025

Choose a reason for hiding this comment

Uh oh!

vraja-pro Jan 6, 2025

Choose a reason for hiding this comment

Uh oh!

igorschoester Jan 7, 2025

Choose a reason for hiding this comment

Uh oh!

vraja-pro Jan 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

igorschoester Apr 14, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

vraja-pro commented Apr 8, 2025

Uh oh!

vraja-pro commented Apr 8, 2025

Uh oh!

igorschoester left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

igorschoester commented Apr 14, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

vraja-pro commented Dec 31, 2024 •

edited

Loading

coveralls commented Dec 31, 2024 •

edited

Loading

igorschoester left a comment •

edited

Loading

vraja-pro Jan 14, 2025 •

edited

Loading

igorschoester left a comment •

edited

Loading