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

Add default as prop to PluginMoreMenuItem component #39517

Conversation

AnthonyLedesma
Copy link
Contributor

closes #39474

What?

This PR resolves the issue reported in #37474 by providing a default as prop of the MenuItem component. This solution allows users to continue to override the as prop just as behavior was previously.

Why?

By being able to use the MenuItem we gain the benefits of existing work done to handle aria attributes for example among other benefits such as styling consistency.

How?

Minimal JavaScript changes to this package. Should only affect custom menu elements in the editor.

Testing Instructions

With this PR the following snippet will render a MenuItem as explained in the PluginMoreMenuItem docs

import { PluginMoreMenuItem } from '@wordpress/edit-post';

...
	return (
		<>
			<PluginMoreMenuItem  onClick={ openModal }>
				{ __( 'Editor settings', 'coblocks' ) }
			</PluginMoreMenuItem>
			...
		</>
        )

Screenshots or screencast

image

@Mamaduka Mamaduka added [Feature] Extensibility The ability to extend blocks or the editing experience [Type] Bug An existing feature does not function as intended [Package] Edit Post /packages/edit-post labels Mar 17, 2022
@Mamaduka Mamaduka requested a review from talldan March 17, 2022 05:18
Copy link
Contributor

@talldan talldan left a comment

Choose a reason for hiding this comment

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

This makes sense to me, thanks for the PR.

There is also a PluginMoreMenuItem in the edit-site package:
https://github.com/WordPress/gutenberg/blob/trunk/packages/edit-site/src/components/header/plugin-more-menu-item/index.js

It would be great to make that consistent before we merge this.

@AnthonyLedesma
Copy link
Contributor Author

There is also a PluginMoreMenuItem in the edit-site package:
https://github.com/WordPress/gutenberg/blob/trunk/packages/edit-site/src/components/header/plugin-more-menu-item/index.js

I went ahead and pushed the update to the edit-site package as well.

Copy link
Contributor

@talldan talldan left a comment

Choose a reason for hiding this comment

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

Thanks for fixing this 🎉

@talldan talldan merged commit d799a96 into WordPress:trunk Mar 18, 2022
@github-actions github-actions bot added this to the Gutenberg 12.9 milestone Mar 18, 2022
@gziolo
Copy link
Member

gziolo commented Mar 22, 2022

Great fix, thank you 🙇🏻

jostnes pushed a commit to jostnes/gutenberg that referenced this pull request Mar 23, 2022
)

* add default as prop

* also update edit-site package
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Extensibility The ability to extend blocks or the editing experience [Package] Edit Post /packages/edit-post [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

PluginMoreMenuItem implementation does not render MenuItem as expected
4 participants