Skip to content

Allow embed sandbox to make content non-interactive for unknown embeds #10512

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.

Sign up for GitHub

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

Closed
notnownikki wants to merge 1 commit into from
Closed

Allow embed sandbox to make content non-interactive for unknown embeds #10512

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 2 additions & 0 deletions packages/block-library/src/embed/core-embeds.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,7 @@ export const common = [
icon: embedVimeoIcon,
keywords: [ __( 'video' ) ],
description: __( 'Embed a Vimeo video.' ),
interactive: false,
},
patterns: [ /^https?:\/\/(www\.)?vimeo\.com\/.+/i ],
},
Expand Down Expand Up @@ -177,6 +178,7 @@ export const others = [
title: 'Imgur',
icon: embedPhotoIcon,
description: __( 'Embed Imgur content.' ),
interactive: false,
},
patterns: [ /^https?:\/\/(.+\.)?imgur\.com\/.+/i ],
},
Expand Down
3 changes: 2 additions & 1 deletion packages/block-library/src/embed/edit.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import { kebabCase, toLower } from 'lodash';
import { __, sprintf } from '@wordpress/i18n';
import { Component, Fragment } from '@wordpress/element';

export function getEmbedEditComponent( title, icon, responsive = true ) {
export function getEmbedEditComponent( title, icon, responsive = true, interactive = true ) {
return class extends Component {
constructor() {
super( ...arguments );
Expand Down Expand Up @@ -186,6 +186,7 @@ export function getEmbedEditComponent( title, icon, responsive = true ) {
isSelected={ isSelected }
icon={ icon }
label={ label }
interactive={ interactive }
/>
</Fragment>
);
Expand Down
3 changes: 2 additions & 1 deletion packages/block-library/src/embed/embed-preview.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import { RichText, BlockIcon } from '@wordpress/editor';
import WpEmbedPreview from './wp-embed-preview';

const EmbedPreview = ( props ) => {
const { preview, url, type, caption, onCaptionChange, isSelected, className, icon, label } = props;
const { preview, url, type, caption, onCaptionChange, isSelected, className, icon, label, interactive } = props;
const { scripts } = preview;

const html = 'photo' === type ? getPhotoHtml( preview ) : preview.html;
Expand All @@ -45,6 +45,7 @@ const EmbedPreview = ( props ) => {
scripts={ scripts }
title={ iframeTitle }
type={ sandboxClassnames }
interactive={ interactive }
Copy link
Member

@aduth aduth Nov 19, 2018

Choose a reason for hiding this comment

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

Out of curiosity, is there any reason we couldn't use Disabled for this?

https://github.com/WordPress/gutenberg/tree/master/packages/components/src/disabled

/>
</div>
);
Expand Down
1 change: 1 addition & 0 deletions packages/block-library/src/embed/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ export const settings = getEmbedBlockSettings( {
},
],
},
interactive: false,
} );

export const common = commonEmbeds.map(
Expand Down
4 changes: 2 additions & 2 deletions packages/block-library/src/embed/settings.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,10 @@ const embedAttributes = {
},
};

export function getEmbedBlockSettings( { title, description, icon, category = 'embed', transforms, keywords = [], supports = {}, responsive = true } ) {
export function getEmbedBlockSettings( { title, description, icon, category = 'embed', transforms, keywords = [], supports = {}, responsive = true, interactive = true } ) {
// translators: %s: Name of service (e.g. VideoPress, YouTube)
const blockDescription = description || sprintf( __( 'Add a block that displays content pulled from other sites, like Twitter, Instagram or YouTube.' ), title );
const edit = getEmbedEditComponent( title, icon, responsive );
const edit = getEmbedEditComponent( title, icon, responsive, interactive );
return {
title,
description: blockDescription,
Expand Down
13 changes: 12 additions & 1 deletion packages/components/src/sandbox/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,8 +154,17 @@ class Sandbox extends Component {
margin-top: 0 !important; /* Has to have !important to override inline styles. */
margin-bottom: 0 !important;
}
#wp-sandbox-overlay {
position: fixed;
width: 100%;
height: 100%;
top: 0;
left: 0;
right: 0;
bottom: 0;
}
`;

const overlayId = 'wp-sandbox-overlay';
Copy link
Member

@aduth aduth Nov 19, 2018

Choose a reason for hiding this comment

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

I'm not sure I see a benefit to assigning this to a variable if it's not in-fact a single source of truth (i.e. duplicated in the style assignment).

Copy link
Member Author

@notnownikki notnownikki Dec 17, 2018

Choose a reason for hiding this comment

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

Me neither, but the linter doesn't allow a string literal as id :)

// put the html snippet into a html document, and then write it to the iframe's document
// we can use this in the future to inject custom styles or scripts.
// Scripts go into the body rather than the head, to support embedded content such as Instagram
Expand All @@ -167,6 +176,7 @@ class Sandbox extends Component {
<style dangerouslySetInnerHTML={ { __html: style } } />
</head>
<body data-resizable-iframe-connected="data-resizable-iframe-connected" className={ this.props.type }>
{ ! this.props.interactive && <div id={ overlayId } /> }
<div dangerouslySetInnerHTML={ { __html: this.props.html } } />
<script type="text/javascript" dangerouslySetInnerHTML={ { __html: observeAndResizeJS } } />
{ ( this.props.scripts && this.props.scripts.map(
Expand All @@ -189,6 +199,7 @@ class Sandbox extends Component {
return {
html: '',
title: '',
interactive: true,
};
}

Expand Down