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

Gallery: Fix regression in conversion from shortcode #4818

Merged
merged 1 commit into from
Feb 2, 2018

Conversation

mcsf
Copy link
Contributor

@mcsf mcsf commented Feb 1, 2018

Description

Fixes a regression, introduced in #4567, whereby pasting a gallery shortcode ([gallery ids="1,2,3"]) would throw a TypeError and break the paste action.

A gallery's images attribute may contain images missing a url. Notably, when creating a block from the pasting of a gallery shortcode ([gallery ids="1,2,3"]), the editor needs to work with media IDs to retrieve the corresponding URLs. Provisions are made in order to support optional url data in GalleryImage:

componentWillReceiveProps( { image } ) {
if ( image && image.data && ! this.props.url ) {
this.props.setAttributes( {
url: image.data.source_url,
alt: image.data.alt_text,

const img = url ? <img src={ url } alt={ alt } data-id={ id } /> : <Spinner />;

This PR applies the same to the new logic that handles transient media objects.

How Has This Been Tested?

Screenshots (jpeg or gifs if applicable):

Types of changes

  • Bug fix

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code has proper inline documentation.

@mcsf mcsf added [Type] Bug An existing feature does not function as intended [Feature] Paste labels Feb 1, 2018
@mcsf mcsf requested a review from youknowriad February 1, 2018 23:27
Copy link
Contributor

@youknowriad youknowriad left a comment

Choose a reason for hiding this comment

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

LGTM 👍 I noticed an issue with invalid galleries when using "Link To Attachment" and saving and refreshing the page. But I believe it's not related to this PR and more related to the fact that we're not persisting the "link" attribute of the images.

@mcsf mcsf merged commit b02eb78 into master Feb 2, 2018
@youknowriad youknowriad deleted the fix/gallery-shortcode-typeerror branch February 2, 2018 10:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Paste [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants