Fix property name in PluginBlockSettingsMenuItem

[swissspidy]

Description

In #7895, PluginBlockSettingsMenuItem was made extensible for plugin developers.

There was a slight oversight though regarding the allowedBlockNames prop. While the documentation was referring to allowedBlockNames, the code was actually checking for allowedBlocks,

How has this been tested?

I extended the settings menu by using the examples from the documentation and verified that allowedBlocks now is properly documented as such.

Screenshots

Types of changes

Bug fix (non-breaking change which fixes an issue)

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.

[oandregal]

In my own testing example, I used allowedBlocks 🤦‍♂️

Thanks for catching this @swissspidy I think it'll be better to fix it the other way round: rename the allowBlockNames instances to allowBlocks, as that's what we use in other places of the codebase for allowed lists (filters, innerBlocks, columns, media-text, etc.).

[aduth]

Bug fix (non-breaking change which fixes an issue)

Wouldn't it be considered breaking for those which would have used allowedBlocks from how it had been implemented in the code?

[swissspidy]

Wouldn't it be considered breaking for those which would have used allowedBlocks from how it had been implemented in the code?

Indeed, but I am not sure how many people that really did. I'd say most people go by the documentation, which means changing everything to allowBlocks would be breaking as well.

I suppose we could just do allowBlockNames || allowBlocks for BC?

[aduth]

which means changing everything to allowBlocks would be breaking as well.

Is it breaking if it never worked? 😆

[swissspidy]

Fair enough :-)

I'll change it to allowedBlocks then.

[aduth]

rename the allowBlockNames instances to allowBlocks, as that's what we use in other places of the codebase for allowed lists (filters, innerBlocks, columns, media-text, etc.).

Frustratingly, it's not all consistent. The BlockEditorProvider component, for example, receives allowedBlockTypes as a setting.

I'd say as far as not creating further inconsistency, allowedBlocks would be the best option here. There's no other existing occurrence of allowedBlockNames to use as precedent.

I guess the documentation may have been misled by the argument of this function?

gutenberg/packages/edit-post/src/components/block-settings-menu/plugin-block-settings-menu-item.js

Lines 26 to 30 in ad7d183

	* @param {string[]} selectedBlockNames Array containing the names of the blocks selected
	* @param {string[]} allowedBlockNames Array containing the names of the blocks allowed
	* @return {boolean} Whether the item will be rendered or not.
	*/
	const shouldRenderItem = ( selectedBlockNames, allowedBlockNames ) => ! Array.isArray( allowedBlockNames ) ||

[oandregal]

It'll be nice if we changed the shouldRenderItem( selectedBlockNames, allowedBlockNames ) to shouldRenderItem( selectedBlocks, allowedBlocks ) as per internal consistency.