Ensure correct attributes are always used for embed preview (#14748)

* Ensure correct attributes are always used for embed preview * Docs update * Update missed call * Fix up attributes function names, clarify render comment
WordPress · May 9, 2019 · f628a8a · f628a8a
1 parent 5924233
commit f628a8a
Show file tree

Hide file tree

Showing 2 changed files with 67 additions and 41 deletions.
diff --git a/packages/block-library/src/embed/edit.js b/packages/block-library/src/embed/edit.js
@@ -1,7 +1,7 @@
 /**
  * Internal dependencies
  */
-import { isFromWordPress, createUpgradedEmbedBlock, getClassNames, fallback } from './util';
+import { createUpgradedEmbedBlock, getClassNames, fallback, getAttributesFromPreview } from './util';
 import EmbedControls from './embed-controls';
 import EmbedLoading from './embed-loading';
 import EmbedPlaceholder from './embed-placeholder';
@@ -10,7 +10,7 @@ import EmbedPreview from './embed-preview';
 /**
  * External dependencies
  */
-import { kebabCase, toLower } from 'lodash';
+import classnames from 'classnames';
 
 /**
  * WordPress dependencies
@@ -24,8 +24,8 @@ export function getEmbedEditComponent( title, icon, responsive = true ) {
  super( ...arguments );
  this.switchBackToURLInput = this.switchBackToURLInput.bind( this );
  this.setUrl = this.setUrl.bind( this );
- this.getAttributesFromPreview = this.getAttributesFromPreview.bind( this );
- this.setAttributesFromPreview = this.setAttributesFromPreview.bind( this );
+ this.getMergedAttributes = this.getMergedAttributes.bind( this );
+ this.setMergedAttributes = this.setMergedAttributes.bind( this );
  this.getResponsiveHelp = this.getResponsiveHelp.bind( this );
  this.toggleResponsive = this.toggleResponsive.bind( this );
  this.handleIncomingPreview = this.handleIncomingPreview.bind( this );
@@ -41,11 +41,10 @@ export function getEmbedEditComponent( title, icon, responsive = true ) {
  }
 
  handleIncomingPreview() {
- const { allowResponsive } = this.props.attributes;
- this.setAttributesFromPreview();
+ this.setMergedAttributes();
  const upgradedBlock = createUpgradedEmbedBlock(
  this.props,
- this.getAttributesFromPreview( this.props.preview, allowResponsive )
+ this.getMergedAttributes()
  );
  if ( upgradedBlock ) {
  this.props.onReplace( upgradedBlock );
@@ -90,42 +89,20 @@ export function getEmbedEditComponent( title, icon, responsive = true ) {
  }
 
  /***
- * Gets block attributes based on the preview and responsive state.
- *
- * @param {string} preview The preview data.
- * @param {boolean} allowResponsive Apply responsive classes to fixed size content.
- * @return {Object} Attributes and values.
+ * @return {Object} Attributes derived from the preview, merged with the current attributes.
  */
- getAttributesFromPreview( preview, allowResponsive = true ) {
- const attributes = {};
- // Some plugins only return HTML with no type info, so default this to 'rich'.
- let { type = 'rich' } = preview;
- // If we got a provider name from the API, use it for the slug, otherwise we use the title,
- // because not all embed code gives us a provider name.
- const { html, provider_name: providerName } = preview;
- const providerNameSlug = kebabCase( toLower( '' !== providerName ? providerName : title ) );
-
- if ( isFromWordPress( html ) ) {
- type = 'wp-embed';
- }
-
- if ( html || 'photo' === type ) {
- attributes.type = type;
- attributes.providerNameSlug = providerNameSlug;
- }
-
- attributes.className = getClassNames( html, this.props.attributes.className, responsive && allowResponsive );
-
- return attributes;
+ getMergedAttributes() {
+ const { preview } = this.props;
+ const { className, allowResponsive } = this.props.attributes;
+ return { ...this.props.attributes, ...getAttributesFromPreview( preview, title, className, responsive, allowResponsive ) };
  }
 
  /***
- * Sets block attributes based on the preview data.
+ * Sets block attributes based on the current attributes and preview data.
  */
- setAttributesFromPreview() {
- const { setAttributes, preview } = this.props;
- const { allowResponsive } = this.props.attributes;
- setAttributes( this.getAttributesFromPreview( preview, allowResponsive ) );
+ setMergedAttributes() {
+ const { setAttributes } = this.props;
+ setAttributes( this.getMergedAttributes() );
  }
 
  switchBackToURLInput() {
@@ -151,8 +128,7 @@ export function getEmbedEditComponent( title, icon, responsive = true ) {
 
  render() {
  const { url, editingURL } = this.state;
- const { caption, type, allowResponsive } = this.props.attributes;
- const { fetching, setAttributes, isSelected, className, preview, cannotEmbed, themeSupportsResponsive, tryAgain } = this.props;
+ const { fetching, setAttributes, isSelected, preview, cannotEmbed, themeSupportsResponsive, tryAgain } = this.props;
 
  if ( fetching ) {
  return (
@@ -179,6 +155,18 @@ export function getEmbedEditComponent( title, icon, responsive = true ) {
  );
  }
 
+ // Even though we set attributes that get derived from the preview,
+ // we don't access them directly because for the initial render,
+ // the `setAttributes` call will not have taken effect. If we're
+ // rendering responsive content, setting the responsive classes
+ // after the preview has been rendered can result in unwanted
+ // clipping or scrollbars. The `getAttributesFromPreview` function
+ // that `getMergedAttributes` uses is memoized so that we're not
+ // calculating them on every render.
+ const previewAttributes = this.getMergedAttributes();
+ const { caption, type, allowResponsive } = previewAttributes;
+ const className = classnames( previewAttributes.className, this.props.className );
+
  return (
  <>
  <EmbedControls

diff --git a/packages/block-library/src/embed/util.js b/packages/block-library/src/embed/util.js
@@ -7,8 +7,9 @@ import { DEFAULT_EMBED_BLOCK, WORDPRESS_EMBED_BLOCK, ASPECT_RATIOS } from './con
 /**
  * External dependencies
  */
-import { includes } from 'lodash';
+import { includes, kebabCase, toLower } from 'lodash';
 import classnames from 'classnames/dedupe';
+import memoize from 'memize';
 
 /**
  * WordPress dependencies
@@ -178,3 +179,40 @@ export function fallback( url, onReplace ) {
  createBlock( 'core/paragraph', { content: renderToString( link ) } )
  );
 }
+
+/***
+ * Gets block attributes based on the preview and responsive state.
+ *
+ * @param {Object} preview The preview data.
+ * @param {string} title The block's title, e.g. Twitter.
+ * @param {Object} currentClassNames The block's current class names.
+ * @param {boolean} isResponsive Boolean indicating if the block supports responsive content.
+ * @param {boolean} allowResponsive Apply responsive classes to fixed size content.
+ * @return {Object} Attributes and values.
+ */
+export const getAttributesFromPreview = memoize( ( preview, title, currentClassNames, isResponsive, allowResponsive = true ) => {
+ if ( ! preview ) {
+ return {};
+ }
+
+ const attributes = {};
+ // Some plugins only return HTML with no type info, so default this to 'rich'.
+ let { type = 'rich' } = preview;
+ // If we got a provider name from the API, use it for the slug, otherwise we use the title,
+ // because not all embed code gives us a provider name.
+ const { html, provider_name: providerName } = preview;
+ const providerNameSlug = kebabCase( toLower( '' !== providerName ? providerName : title ) );
+
+ if ( isFromWordPress( html ) ) {
+ type = 'wp-embed';
+ }
+
+ if ( html || 'photo' === type ) {
+ attributes.type = type;
+ attributes.providerNameSlug = providerNameSlug;
+ }
+
+ attributes.className = getClassNames( html, currentClassNames, isResponsive && allowResponsive );
+
+ return attributes;
+} );