Directory

Add code indentation and escape image links by kishanjasani · Pull Request #66 · WordPress/twentytwentyfour · GitHub
Skip to content
This repository has been archived by the owner on Jan 24, 2024. It is now read-only.

Add code indentation and escape image links #66

Merged

Conversation

kishanjasani
Copy link
Contributor

This addresses issue #65 and adds translation functions to text in the call to action pattern.

@carolinan
Copy link
Contributor

Hi!
Thank you for the update.
The themes use echo because each of the example texts needs to have the translator's context added. We did not bother adding that everywhere during the in-person contributor day as people were learning, and the time was limited.

So esc_html_e() or echo esc_html__() needs to be echo esc_html_x( 'text', 'context', 'text-domain' )

Here are examples from Twenty Twenty Three:

<!-- wp:paragraph {"style":{"typography":{"lineHeight":"1.2"}},"fontSize":"x-large"} -->
<p class="has-x-large-font-size" style="line-height:1.2"><?php echo esc_html_x( 'Got any book recommendations?', 'sample content for call to action', 'twentytwentythree' ); ?></p>
<!-- /wp:paragraph -->

<!-- wp:paragraph -->
<p><?php echo esc_html_x( 'This page could not be found.', 'Message to convey that a webpage could not be found', 'twentytwentythree' ); ?></p>
<!-- /wp:paragraph -->

@kishanjasani
Copy link
Contributor Author

@carolinan Thanks for the feedback! I will update it with echo esc_html_x().

@kishanjasani
Copy link
Contributor Author

@carolinan Updated the code as per your suggestions. Let me know if I missed something.

@MaggieCabrera
Copy link
Collaborator

Sadly I think this needs a rebase, are you ok to do this?

@luminuu
Copy link
Member

luminuu commented Sep 5, 2023

@luminuu
Copy link
Member

luminuu commented Sep 12, 2023

@kishanjasani Are you able to update your PR? Otherwise I'd create a new one with your changes there, as I can't update your PR unfortunately.

@MaggieCabrera
Copy link
Collaborator

@kishanjasani Are you able to update your PR? Otherwise I'd create a new one with your changes there, as I can't update your PR unfortunately.

@luminuu I'm going to check your permissions, but I've been able to alter everyone's PRs using GH cli

@kishanjasani
Copy link
Contributor Author

@luminuu I am on it.

@kishanjasani
Copy link
Contributor Author

kishanjasani commented Sep 12, 2023

Hi @luminuu and @MaggieCabrera I have rebase the code. Thanks

Copy link
Contributor

@carolinan carolinan left a comment

Choose a reason for hiding this comment

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

Note: There are several open pull requests that are updating this file, so some of these changes are likely to be overridden again the next few days.

@carolinan carolinan merged commit f10f38e into WordPress:trunk Sep 14, 2023
2 checks passed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants