Skip to content
This repository has been archived by the owner on Nov 18, 2024. It is now read-only.

Try: Escape apostrophe in translation functions. #537

Merged
merged 6 commits into from
Oct 16, 2024

Conversation

juanfra
Copy link
Member

@juanfra juanfra commented Oct 9, 2024

Description

Replacing ’ for an escaped apostrophe (\') in translatable texts.

Screenshots

  • N/A

Testing Instructions

  1. Create a page.
  2. Insert all the patterns modified in this PR.
  3. Confirm the texts are looking as they were before.

@juanfra juanfra added [Type] Enhancement A suggestion for improvement. Internationalization (i18n) Issues or PRs related to internationalization efforts labels Oct 9, 2024
@juanfra juanfra requested a review from carolinan October 9, 2024 16:33
@juanfra juanfra self-assigned this Oct 9, 2024
Copy link

github-actions bot commented Oct 9, 2024

Preview changes

You can preview these changes by following the link below:

I will update this comment with the latest preview links as you push more changes to this PR.
⚠️ Note: The preview sites are created using WordPress Playground. You can add content, edit settings, and test the themes as you would on a real site, but please note that changes are not saved between sessions.

Copy link

github-actions bot commented Oct 9, 2024

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: juanfra <[email protected]>
Co-authored-by: SergeyBiryukov <[email protected]>
Co-authored-by: carolinan <[email protected]>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

@carolinan
Copy link
Contributor

Why, what is the benefit of this change?

@juanfra
Copy link
Member Author

juanfra commented Oct 10, 2024

Why, what is the benefit of this change?

From my experience working with translations, I see that many translators are not technical and don't know about these entities. The benefit would be that it'll be easier for translators to do their work.

I believe historically, in core (and other products I've been involved with), the tendency is to use apostrophes rather than entities.

@carolinan
Copy link
Contributor

That sounds very reasonable to me.

It does not match how these symbols have been managed in the previous bundled themes.
I would like to ask @SergeyBiryukov but he is on vacation until next week.

@carolinan
Copy link
Contributor

carolinan commented Oct 10, 2024

I also recognize that on trunk there seems to be a mix of slashes and entities which was not intended :)

Copy link
Member

@SergeyBiryukov SergeyBiryukov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR! I would say these are fine to replace, and I see that some of the previous bundled themes already use the actual double quote characters instead of entities.

However, entities are generally used for better typography, so instead of the escaped apostrophe character, I would suggest using an actual single quote, which would not need escaping and also has a precedent in the previous bundled themes.

Left some suggestions, hope this helps 🙂

patterns/banner-about-book.php Outdated Show resolved Hide resolved
patterns/banner-poster.php Outdated Show resolved Hide resolved
patterns/event-schedule.php Outdated Show resolved Hide resolved
patterns/page-cv-bio.php Outdated Show resolved Hide resolved
patterns/page-link-in-bio-wide-margins.php Outdated Show resolved Hide resolved
patterns/page-link-in-bio-with-tight-margins.php Outdated Show resolved Hide resolved
juanfra and others added 2 commits October 16, 2024 10:33
@juanfra
Copy link
Member Author

juanfra commented Oct 16, 2024

Thank you both!

@juanfra juanfra merged commit 71b76fe into trunk Oct 16, 2024
4 checks passed
@juanfra juanfra deleted the try/escape-apostrophe-translations branch October 16, 2024 08:38
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Internationalization (i18n) Issues or PRs related to internationalization efforts [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants