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
Fix no-preview placeholder BlockIcon usage #9466
Fix no-preview placeholder BlockIcon usage #9466
Conversation
Resolves: #9444 |
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.
Looks good to me I think we just need to move the translator comment before the merge.
@@ -188,10 +188,10 @@ export function getEmbedEdit( title, icon ) { | |||
); | |||
} | |||
|
|||
const label = sprintf( __( '%s URL' ), title ); | |||
|
|||
if ( ! preview || previewIsFallback || editingURL ) { | |||
// translators: %s: type of embed e.g: "YouTube", "Twitter", etc. "Embed" is used when no specific type exists |
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 think the translator comment also needs to be moved to be in the line before the label.
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.
Gah, I missed that one! Thanks :)
@@ -237,7 +237,7 @@ export function getEmbedEdit( title, icon ) { | |||
<figure className={ classnames( className, 'wp-block-embed', { 'is-video': 'video' === type } ) }> | |||
{ controls } | |||
{ ( cannotPreview ) ? ( | |||
<Placeholder icon={ icon } label={ __( 'Embed URL' ) }> | |||
<Placeholder icon={ <BlockIcon icon={ icon } showColors /> } label={ label } className="wp-block-embed"> |
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.
Why are we duplicating the class
attribute here (it's already on the figure
above)? cc @notnownikki @jorgefilipecosta
Description
Update the Placeholder used to URLs that can be embedded but can't be previewed in the editor due to sandbox security settings.
How has this been tested?
Create a new post, try to embed https://www.facebook.com/buzzfeedladylike/posts/2290362924583261
Types of changes
Bug fix
Checklist: