-
Notifications
You must be signed in to change notification settings - Fork 843
Forms: Add emoji flag to email responses #45645
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds country flag emoji indicators to form submission notification emails by implementing a method to convert country codes to their corresponding emoji flags and displaying them next to the IP address.
- Implements
get_country_flag()andcountry_code_to_emoji_flag()methods in the Feedback class - Updates email template to include flag emoji after IP address
- Adds comprehensive test coverage for various country codes
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| projects/plugins/jetpack/changelog/add-emoji-flag-to-email-responses | Changelog entry documenting the enhancement |
| projects/packages/forms/tests/php/contact-form/Feedback_Test.php | Test cases for country flag conversion with multiple country codes |
| projects/packages/forms/src/contact-form/class-feedback.php | Core implementation of country code to emoji flag conversion |
| projects/packages/forms/src/contact-form/class-contact-form.php | Email template update to display flag emoji next to IP address |
| projects/packages/forms/changelog/add-emoji-flag-to-email-responses | Package-specific changelog entry |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Are you an Automattician? Please test your changes on all WordPress.com environments to help mitigate accidental explosions.
Interested in more tips and information?
|
|
Thank you for your PR! When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:
This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖 Follow this PR Review Process:
If you have questions about anything, reach out in #jetpack-developers for guidance! Jetpack plugin: The Jetpack plugin has different release cadences depending on the platform:
If you have any questions about the release process, please ask in the #jetpack-releases channel on Slack. |
Code Coverage SummaryCoverage changed in 2 files.
|
Introduces a method to convert country codes to emoji flags in the Feedback class and displays the flag alongside the IP address in the contact form footer. Includes unit tests to verify correct flag generation for various country codes and handles cases where the country code is unavailable.
aaf7991 to
0986d98
Compare
| /* translators: Placeholder is the IP address of the person who submitted a form. */ | ||
| esc_html__( 'IP Address: %1$s', 'jetpack-forms' ), | ||
| $comment_author_ip | ||
| /* translators: %1$s placeholder is the IP address, %2$s is the country flag of the person who submitted a form. */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems a bit complicated to have separate placeholder for countryflag, instead of just using one placeholder for both — I don't really see translators needing to swap their placess after all
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, another consideration is that we'll want to link the IP but not the flag, so they'll need to be separately eventually. 🙃 That wouldn't necessarily affect IP Address: %s string tho.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good. I was thinking about this as well. I though that in some languages they would want to have a space in there and it some cases this would not be ideal.
But lets preserve the current translation for this for now and if we get feedback that this doesn't work we can revisit this. ( I will revert to the previous translation )
| "[contact-field label='Name' type='name' required='1'/][contact-field label='Email' type='email' required='1'/][contact-field label='Message' type='textarea' required='1'/]" | ||
| ); | ||
|
|
||
| // Test valid country codes |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you know that all our country codes (including the ones for small countries) map 1:1 to Unicode?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good question. I think so see https://en.wikipedia.org/wiki/Regional_indicator_symbol
I haven't tested this explicitly but that is what I understand. They had to make this mapping somehow ( assigning a unicode to a string ) it would make sense if they followed some sort of convention.
|
|
||
| return $flag; | ||
| } | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did not know the country code emoji's worked like this. :)
edanzer
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.

Part of FORMS-316
This PR adds the emoji flag next the IP Address.
Proposed changes:
get_country_flag()class to the Feedback class.Other information:
Jetpack product discussion
N/A
Does this pull request change what data or activity we track or use?
No
Testing instructions:
Do the tests pass?