-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Make block more menu transformations consistent #5606
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,7 +7,7 @@ import { noop } from 'lodash'; | |
/** | ||
* WordPress dependencies | ||
*/ | ||
import { __, sprintf } from '@wordpress/i18n'; | ||
import { __ } from '@wordpress/i18n'; | ||
import { IconButton, withContext } from '@wordpress/components'; | ||
import { getPossibleBlockTransformations, switchToBlockType } from '@wordpress/blocks'; | ||
import { compose, Fragment } from '@wordpress/element'; | ||
|
@@ -27,9 +27,12 @@ function BlockTransformations( { blocks, small = false, onTransform, onClick = n | |
return ( | ||
<Fragment> | ||
<div className="editor-block-settings-menu__separator" /> | ||
<span | ||
className="editor-block-switcher__menu-title" | ||
> | ||
{ __( 'Transform into:' ) } | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we preserve the translators comment? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I felt like it was unnecessary given the label was changed from sprintf( __( 'Turn into %s' ), title ) into __( 'Transform into:' ) — and also because I can't show the translator comment in context of the label because it's no longer a variable that gets inserted. |
||
</span> | ||
{ possibleBlockTransformations.map( ( { name, title, icon } ) => { | ||
/* translators: label indicating the transformation of a block into another block */ | ||
const shownText = sprintf( __( 'Turn into %s' ), title ); | ||
return ( | ||
<IconButton | ||
key={ name } | ||
|
@@ -39,9 +42,9 @@ function BlockTransformations( { blocks, small = false, onTransform, onClick = n | |
onClick( event ); | ||
} } | ||
icon={ icon } | ||
label={ small ? shownText : undefined } | ||
label={ small ? title : undefined } | ||
> | ||
{ ! small && shownText } | ||
{ ! small && title } | ||
</IconButton> | ||
); | ||
} ) } | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you using this class for something? We try not to inherit styles by reusing classes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did use it to inherit, indeed. Pushed a change which simply duplicates the styles, but I'm not sure that's the best way to do it either.
It seems like the More Menu, and the Block More Menu (block ellipsis menu) and the Block Switcher should all use a generic Menu component. But I'm not sure I have the skills necessary to make that happen, or whether it should happen in this PR. If yes then feel free to push to this branch.