Skip to content
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

Fix skipped shortcode transforms in raw handling #22840

Merged
merged 4 commits into from
Oct 19, 2020

Conversation

apmatthews
Copy link
Contributor

@apmatthews apmatthews commented Jun 2, 2020

Fixes #22837

Description

Changes the shortcode converter in rawHandler() to allow non-linear shortcode replacement while segmenting an HTML string. This prevents the priority or registration order of a transform from affecting how shortcodes are parsed out of HTML and into blocks.

I'd appreciate feedback on whether this might have unintended consequences or if there was a reason it was not done before.

How has this been tested?

I've tested locally with the example outlined in #22837, but someone more familiar with the codebase should definitely take a look.

Integration test case has been included.

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.
  • I've updated all React Native files affected by any refactorings/renamings in this PR.

@ellatrix
Copy link
Member

ellatrix commented Jun 6, 2020

Could you add a test for this? :)

@ellatrix ellatrix requested a review from mcsf June 6, 2020 14:04
@apmatthews
Copy link
Contributor Author

@ellatrix The test case has been added. I think I found the right place for it, but I'm new to contributing here so please let me know if I've missed something. 🙂

@apmatthews
Copy link
Contributor Author

It doesn't seem like the test failures are related to this PR. I rebased on master again today and got a different set of failures than when I rebased in September.

Anything else I can do help this land in WP 5.6?

Copy link
Contributor

@mcsf mcsf left a comment

Choose a reason for hiding this comment

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

The fix seems sound and the tests catch the issue.

@mcsf mcsf merged commit 84ae4d4 into WordPress:master Oct 19, 2020
@mcsf
Copy link
Contributor

mcsf commented Oct 19, 2020

Thanks for the fix, @apmatthews! This should make its way into 5.6. :)

@github-actions github-actions bot added the First-time Contributor Pull request opened by a first-time contributor to Gutenberg repository label Oct 19, 2020
@github-actions github-actions bot added this to the Gutenberg 9.2 milestone Oct 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
First-time Contributor Pull request opened by a first-time contributor to Gutenberg repository
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Valid shortcode transforms are skipped in rawHandler calls
3 participants