-
Notifications
You must be signed in to change notification settings - Fork 842
Forms: change phone dropdown flags handling #46492
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
…ver swtiching to SVGs
|
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: No scheduled milestone found for this plugin. If you have any questions about the release process, please ask in the #jetpack-releases channel on Slack. |
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 fixes an issue where WordPress emoji replacement scripts would convert flag emoji characters to SVG images, breaking the phone field country selector dropdown in forms. The fix changes how flag updates are handled by using the Interactivity API's data-wp-watch with callbacks that directly modify text node data instead of textContent, thereby avoiding triggering wp-emoji's MutationObserver.
Key Changes:
- Replaced
data-wp-textwithdata-wp-watchcallbacks for flag rendering to bypass wp-emoji interference - Added zero-width space (
​) placeholder to ensure text nodes exist for manipulation - Implemented two new callbacks (
updateSelectedFlagandupdateOptionFlag) that modifyfirstChild.datainstead oftextContent
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
projects/packages/forms/src/modules/field-phone/view.js |
Added two new callbacks (updateSelectedFlag and updateOptionFlag) that update flag display by modifying text node data to avoid triggering wp-emoji's MutationObserver |
projects/packages/forms/src/contact-form/class-contact-form-field.php |
Updated phone field combobox HTML to use data-wp-watch with callbacks instead of data-wp-text, added wp-exclude-emoji class and zero-width space placeholder for flag elements |
projects/packages/forms/changelog/fix-forms-phone-dropdown-flags-handling |
Added changelog entry documenting the fix for phone combobox flag handling |
Code Coverage SummaryCoverage changed in 1 file.
Full summary · PHP report · JS report Coverage check overridden by
Coverage tests to be added later
|
dhasilva
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.
LGTM, tests well
Part of FORMS-478
Proposed changes:
When WP emoji replacement scripts swaps our flag characters the interactivity API fails to bind with
data-wp-textand the flags don't update.Summary of the change:
- Replaced data-wp-text with data-wp-watch="callbacks.updateSelectedFlag/updateOptionFlag"
- Added (zero-width space) placeholder to ensure a text node exists
- Kept wp-exclude-emoji class as fallback for initial page parse
- Added updateSelectedFlag and updateOptionFlag callbacks
- These callbacks modify firstChild.data instead of textContent
- Since wp-emoji's MutationObserver only watches childList (not characterData), modifying .data doesn't trigger emoji conversion
The key insight: wp-emoji's MutationObserver uses: .observe(document.body, { childList: true, subtree: true })
Other information:
Jetpack product discussion
Does this pull request change what data or activity we track or use?
No
Testing instructions:
Before the fix:
After the fix:
NOTE: in order to see the swap on local envs, you can use this on startup: