fix: allow dollar sign in custom blocks#946
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🦋 Changeset detectedLatest commit: 1d9e7f8 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
@nicholasio I chatted with @jacobdubail about this today and I think this is the easiest path forward for my situation. On Digital Democracy the client wants to be able to write out $50 million into a Bill Card custom block. They are the yellow cards right under K12 and Higher Education Bills: https://calmatters.digitaldemocracy.org/issues/education. Because we have these set up as custom blocks with TextareaControl when the data gets processed via the Gutenberg.php file the dollar sign is getting stripped out an converted to an empty space. The hope here is that we can allow dollar signs in this specific use case so that it functions like a core block and doesn't strip that character. I also created this PR wasn't sure which directory is correct for this update: 10up/headstartwp-plugin#1 |
📦 Next.js Bundle Analysis for @10up/wp-nextjs-appThis analysis was generated by the Next.js Bundle Analysis action. 🤖 🎉 Global Bundle Size Decreased
DetailsThe global bundle is the javascript bundle that loads alongside every page. It is in its own category because its impact is much higher - an increase to its size means that every page on your website loads slower, and a decrease means every page loads faster. Any third party scripts you have added directly to your app using the If you want further insight into what is behind the changes, give @next/bundle-analyzer a try! |
📦 Next.js Bundle Analysis for @10up/headstartwpThis analysis was generated by the Next.js Bundle Analysis action. 🤖 This PR introduced no changes to the JavaScript bundle! 🙌 |
📦 Next.js Bundle Analysis for @10up/headstartwpThis analysis was generated by the Next.js Bundle Analysis action. 🤖 This PR introduced no changes to the JavaScript bundle! 🙌 |
📦 Next.js Bundle Analysis for @10up/wp-nextjs-appThis analysis was generated by the Next.js Bundle Analysis action. 🤖 🎉 Global Bundle Size Decreased
DetailsThe global bundle is the javascript bundle that loads alongside every page. It is in its own category because its impact is much higher - an increase to its size means that every page on your website loads slower, and a decrease means every page loads faster. Any third party scripts you have added directly to your app using the If you want further insight into what is behind the changes, give @next/bundle-analyzer a try! |
nicholasio
left a comment
nicholasio
left a comment
There was a problem hiding this comment.
Thanks for the contribution!
Could you add a couple of tests to cover this edge case as well as add a changeset?
|
@nicholasio just added the tests and a .md in the changesets folder. Please let me know if any other updates are needed or if there are different places that I need to update as well. |
📦 Next.js Bundle Analysis for @10up/headstartwpThis analysis was generated by the Next.js Bundle Analysis action. 🤖 This PR introduced no changes to the JavaScript bundle! 🙌 |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Test asserts attribute that method never adds
- Removed the incorrect assertion checking for data-wp-block-name attribute, which is not added by set_block_attributes_tag_api(), allowing the actual dollar-sign preservation test to execute.
Or push these changes by commenting:
@cursor push 0c1158d2b9
Preview (0c1158d2b9)
diff --git a/wp/headless-wp/tests/php/tests/TestGutenbergIntegration.php b/wp/headless-wp/tests/php/tests/TestGutenbergIntegration.php
--- a/wp/headless-wp/tests/php/tests/TestGutenbergIntegration.php
+++ b/wp/headless-wp/tests/php/tests/TestGutenbergIntegration.php
@@ -173,7 +173,6 @@
$enhanced = $this->parser->set_block_attributes_tag_api( $placeholder, $html, $attrs_json );
- $this->assertStringContainsString( 'data-wp-block-name="', $enhanced, 'Sanity check: block-name attribute should exist.' );
// The core assertion: `$50` should not be interpreted by preg_replace() replacement parsing.
$this->assertStringContainsString( '$50 million', $enhanced );
}This Bugbot Autofix run was free. To enable autofix for future PRs, go to the Cursor dashboard.
|
|
||
| $enhanced = $this->parser->set_block_attributes_tag_api( $placeholder, $html, $attrs_json ); | ||
|
|
||
| $this->assertStringContainsString( 'data-wp-block-name="', $enhanced, 'Sanity check: block-name attribute should exist.' ); |
There was a problem hiding this comment.
Test asserts attribute that method never adds
High Severity
The test test_set_block_attributes_tag_api_preserves_dollar_signs asserts that the output contains data-wp-block-name=", but the method being tested (set_block_attributes_tag_api) never adds that attribute. The data-wp-block-name attribute is only set by process_block_with_html_tag_api via WP_HTML_Tag_Processor::set_attribute(). This causes the test to always fail at the "sanity check" assertion on line 176, meaning the actual dollar-sign assertion on line 178 is never reached, making the test unable to verify the fix it was written for.
📦 Next.js Bundle Analysis for @10up/wp-nextjs-appThis analysis was generated by the Next.js Bundle Analysis action. 🤖 🎉 Global Bundle Size Decreased
DetailsThe global bundle is the javascript bundle that loads alongside every page. It is in its own category because its impact is much higher - an increase to its size means that every page on your website loads slower, and a decrease means every page loads faster. Any third party scripts you have added directly to your app using the If you want further insight into what is behind the changes, give @next/bundle-analyzer a try! |
|
You have used all of your free Bugbot PR reviews. To receive reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial. |
📦 Next.js Bundle Analysis for @10up/headstartwpThis analysis was generated by the Next.js Bundle Analysis action. 🤖 This PR introduced no changes to the JavaScript bundle! 🙌 |
📦 Next.js Bundle Analysis for @10up/wp-nextjs-appThis analysis was generated by the Next.js Bundle Analysis action. 🤖 🎉 Global Bundle Size Decreased
DetailsThe global bundle is the javascript bundle that loads alongside every page. It is in its own category because its impact is much higher - an increase to its size means that every page on your website loads slower, and a decrease means every page loads faster. Any third party scripts you have added directly to your app using the If you want further insight into what is behind the changes, give @next/bundle-analyzer a try! |
2 participants
Description of the Change
Client would like to be able to use dollar signs as text within custom blocks. Currently because of preg_replace() dollar signs and the immediate following text are being stripped and convert to empty strings.
How to test the Change
Create a custom block that allows a user to provide string output via a text input or text area. Use a dollar sign in those fields and it should output example: $50
Changelog Entry
Credits
Props @pdavies88
Checklist:
Note
Low Risk
Low risk: small, targeted escaping change to a
preg_replacereplacement string, covered by new regression tests.Overview
Fixes a Gutenberg rendering edge case where
$...sequences inside serialized JSON block attributes were being treated aspreg_replace()replacement tokens and removed duringdata-wp-blockinjection.set_block_attributes_tag_api()now escapes backslashes and dollar signs in the replacement string, and tests add coverage for dollar-sign values (including a new block roundtrip case) plus a changeset entry.Written by Cursor Bugbot for commit 0dc499b. This will update automatically on new commits. Configure here.