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

Make block more menu transformations consistent #5606

Merged
merged 2 commits into from
Mar 21, 2018
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 8 additions & 5 deletions editor/components/block-settings-menu/block-transformations.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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"
Copy link
Member

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.

Copy link
Contributor Author

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.

>
{ __( 'Transform into:' ) }
Copy link
Member

Choose a reason for hiding this comment

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

Should we preserve the translators 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.

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 }
Expand All @@ -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>
);
} ) }
Expand Down