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

Fix the latest-posts block when imageDimensions is empty #21070

Merged
merged 1 commit into from
Apr 2, 2020

Conversation

roo2
Copy link
Contributor

@roo2 roo2 commented Mar 23, 2020

Description

We found a bug when testing the Latest Posts block on WordPress.com that caused the block to fail to load: Automattic/wp-calypso#40127

There was a javascript crash caused by imageDimensions array having missing values for certain sizes that we try to access without a null check via imageDimensions[featuredImageSizeSlug].height

This PR adds a null check and defaults the image size to 0 if not values are found in the imageDimensions array.

How has this been tested?

I tested this by modifying imageDimensions to be empty. The Latest Posts block was then able to load, save, preview, etc.

After applying this fix, I did notice that if you select the named percentage sizes: "25%, 50%, 75%, 100%" then the image's max-width is set to 0px. I'm not sure if that is related but it could be fixed in addition to this null check.

I also noticed that the Width and Height values that the user enters set max-width and max-height css properties. This means that only the lowest value of Width and Height is used, If this is deliberate, I think it should be communicated better to the user :)

@simison simison changed the title fix the latest-posts block for wpcom Fix the latest-posts block Mar 23, 2020
@simison simison added [Block] Latest Posts Affects the Latest Posts Block [Type] Bug An existing feature does not function as intended labels Mar 23, 2020
@simison simison changed the title Fix the latest-posts block Fix the latest-posts block when imageDimensions is empty Mar 23, 2020
Copy link
Member

@noahtallen noahtallen left a comment

Choose a reason for hiding this comment

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

Great change! I think the test failures are affecting all Gutenberg PRs at the moment.

At any rate, it's important for folks to remember that since image dimensions are filterable on the server, it's possible for any of them to be filtered out.

I don't think any of the bugs you noted in the PR are related to these changes since the block has been working in the "normal" case. This doesn't change any of that behavior.

@noahtallen
Copy link
Member

@roo2 could you rebase to try to fix tests? (I can merge once everything passes)

@roo2 roo2 force-pushed the fix/latest-post-block-on-wpcom branch from 3148796 to f7a751a Compare April 1, 2020 03:24
@roo2 roo2 force-pushed the fix/latest-post-block-on-wpcom branch from f7a751a to 9217a2c Compare April 1, 2020 08:47
@roo2
Copy link
Contributor Author

roo2 commented Apr 1, 2020

hey thanks for reminding me, yep the tests are passing now! :)

@noahtallen noahtallen merged commit 1059270 into WordPress:master Apr 2, 2020
@github-actions github-actions bot added the First-time Contributor Pull request opened by a first-time contributor to Gutenberg repository label Apr 2, 2020
@github-actions
Copy link

github-actions bot commented Apr 2, 2020

Congratulations on your first merged pull request! We'd like to credit you for your contribution in the post announcing the next WordPress release, but we can't find a WordPress.org profile associated with your GitHub account. When you have a moment, visit the following URL and click "link your GitHub account" under "GitHub Username" to link your accounts:

https://profiles.wordpress.org/me/profile/edit/

And if you don't have a WordPress.org account, you can create one on this page:

https://login.wordpress.org/register

Kudos!

@github-actions github-actions bot added this to the Gutenberg 7.9 milestone Apr 2, 2020
@jorgefilipecosta jorgefilipecosta added the Backport to WP 6.7 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta label Apr 2, 2020
@whyisjake whyisjake mentioned this pull request Apr 22, 2020
whyisjake pushed a commit that referenced this pull request Apr 22, 2020
Co-authored-by: roo2 <roo2@protonmail.com>
whyisjake pushed a commit that referenced this pull request Apr 23, 2020
Co-authored-by: roo2 <roo2@protonmail.com>
@jorgefilipecosta jorgefilipecosta removed the Backport to WP 6.7 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta label May 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Latest Posts Affects the Latest Posts Block First-time Contributor Pull request opened by a first-time contributor to Gutenberg repository [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants