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

Provide context when blocks.registerBlockType is applied to deprecations #36628

Merged
merged 2 commits into from
May 26, 2022

Conversation

ethitter
Copy link
Contributor

Description

The blocks.registerBlockType filter is applied both to the block's full settings as well as to the settings for its deprecations. If care is not taken, a change intended for the current block's settings can be applied to one of its deprecations unintentionally, with the potential to break the deprecation. This can be avoided by testing the block's settings before applying changes via the filter, but the block's settings may not always be changed in a way that can easily be discerned. To assist with determining the context in which the filter is applied, the deprecation's settings are passed as a third argument, and null is passed when the filter is applied to the block's complete settings.

How has this been tested?

Added a filter that expects three arguments, confirmed that received arguments were as expected.

Screenshots

Types of changes

  • Adds third argument to blocks.registerBlockType that is null when filtering the full settings, and contains a deprecation's settings when applied to one of the block's deprecations.

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • I've tested my changes with keyboard and screen readers.
  • 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 (please manually search all *.native.js files for terms that need renaming or removal).

@gziolo
Copy link
Member

gziolo commented Nov 29, 2021

@youknowriad and @mcsf, what are your thoughts on the proposed approach? I can see how filters mixed with deprecations might cause issues. In particular when a filter gets introduced long after the depreciation was created.

@youknowriad
Copy link
Contributor

I think it's fine yes.

@gziolo gziolo added [Feature] Blocks Overall functionality of blocks [Feature] Block Validation/Deprecation Handling block validation to determine accuracy and deprecation [Feature] Extensibility The ability to extend blocks or the editing experience labels Nov 29, 2021
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.

Let's get this enhancement included. Thank you for the nice idea 👍🏻

@gziolo gziolo merged commit 5b9b8b1 into WordPress:trunk May 26, 2022
@github-actions github-actions bot added this to the Gutenberg 13.4 milestone May 26, 2022
@gziolo gziolo added [Package] Blocks /packages/blocks [Type] Enhancement A suggestion for improvement. labels May 26, 2022
@ethitter ethitter deleted the add/block-type-deprecate-context branch May 26, 2022 15:42
@mcsf
Copy link
Contributor

mcsf commented May 26, 2022

Nice! I missed the ping back in November. Great seeing you here, @ethitter, saudades! 😄

@bph bph added the Needs Dev Note Requires a developer note for a major WordPress release cycle label Jun 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Block Validation/Deprecation Handling block validation to determine accuracy and deprecation [Feature] Blocks Overall functionality of blocks [Feature] Extensibility The ability to extend blocks or the editing experience Needs Dev Note Requires a developer note for a major WordPress release cycle [Package] Blocks /packages/blocks [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants