Skip to content

Conversation

@mcsf
Copy link
Contributor

@mcsf mcsf commented Jan 27, 2018

Fixes regression identified in #4514 (comment) whereby freeform-to-block conversion and pasting of known shortcodes, such as [gallery] was intercepted by the generic Shortcode block.

Description

Read comment in diff for details. The gist is that shortcode conversion doesn't support concurrent shortcode transforms, in that there is no explicit prioritization mechanism to handle conflicting matches. A proper solution is underway, but until then this quick fix restores registration order to its state before #4514.

How Has This Been Tested?

  1. Open Gutenberg.
  2. Paste [gallery] or some custom transform-enabled shortcode.
  3. Make sure that the specific block (e.g. Shortcode) is inserted.
  4. Paste an unknown shortcode ([sfoihergv]).
  5. Make sure that a Shortcode block is inserted.

Types of changes

Bug fix.

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code has proper inline documentation.

- Fixes a regression affecting freeform->block conversion and pasting of
strings such as `[gallery]`.
@mcsf mcsf added [Type] Bug An existing feature does not function as intended [Feature] Paste labels Jan 27, 2018
@mcsf mcsf requested review from ellatrix and gziolo January 27, 2018 11:19
@mcsf mcsf changed the title Prioritize specific (eg Gallery) to Shortcode block during conversion Prioritize specific blocks (e.g. Gallery) over Shortcode block during conversion Jan 27, 2018
// yielding non-deterministic results. A proper solution could be to
// let the editor (or site owners) determine a default block handler of
// unknown shortcodes — see `setUnknownTypeHandlerName`.
shortcode,
Copy link
Member

Choose a reason for hiding this comment

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

How about moving it out of this array to make things extra clear? registerBlockType( shortcode.name, shortcode.settings );?

Copy link
Member

Choose a reason for hiding this comment

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

Both work for me :) Hopefully it will be fixed soon as described in the comment.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure. It feels like all the core blocks should be gathered in a single place, and the current approach of creating an array and looping over it seems more apt. Placing a registerBlockType call for Shortcode beside that loop could perhaps make it less visible?

Copy link
Member

@gziolo gziolo left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@mcsf mcsf merged commit d06f44d into master Jan 27, 2018
@mcsf mcsf deleted the fix/shortcode-conversion-regression branch January 27, 2018 11:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

[Feature] Paste [Type] Bug An existing feature does not function as intended

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants