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

Compat: Conditionally filter editor settings for image dimensions #20939

Merged
merged 3 commits into from
Mar 17, 2020

Conversation

aduth
Copy link
Member

@aduth aduth commented Mar 16, 2020

Fixes #20907
Previously: #17151

This pull request seeks to move gutenberg_extend_settings_image_dimensions to compat.php, considering it as a temporary backporting of behavior otherwise assumed to be handled by WordPress 5.4.0+ (i.e. to support WordPress 5.3.x). In doing so, it makes the implementation more durable by only extending the settings if (a) imageDimensions is not already assigned and (b) the given settings imageSizes is an array.

Testing Instructions:

Repeat Steps to Reproduce from #20907, verifying that no warnings occur.

Repeat Testing Instructions from #17151, ideally both in WordPress 5.3.2 (where the extension will apply) and WordPress 5.4.0 RC2 (where it is not needed).

@aduth aduth added [Type] Bug An existing feature does not function as intended [Type] Plugin Interoperability Incompatibilities between a specific plugin and the block editor. Close with workaround notes. [Block] Latest Posts Affects the Latest Posts Block labels Mar 16, 2020
@aduth aduth requested a review from ryelle March 16, 2020 20:52
@github-actions
Copy link

github-actions bot commented Mar 16, 2020

Size Change: -118 B (0%)

Total Size: 857 kB

Filename Size Change
build/block-editor/index.js 100 kB -85 B (0%)
build/block-library/index.js 111 kB -38 B (0%)
build/components/index.js 191 kB -27 B (0%)
build/components/style-rtl.css 15.7 kB +4 B (0%)
build/components/style.css 15.7 kB +4 B (0%)
build/editor/index.js 44 kB +24 B (0%)
ℹ️ View Unchanged
Filename Size Change
build/a11y/index.js 998 B 0 B
build/annotations/index.js 3.43 kB 0 B
build/api-fetch/index.js 3.39 kB 0 B
build/autop/index.js 2.58 kB 0 B
build/blob/index.js 620 B 0 B
build/block-directory/index.js 6.02 kB 0 B
build/block-directory/style-rtl.css 760 B 0 B
build/block-directory/style.css 760 B 0 B
build/block-editor/style-rtl.css 10.9 kB 0 B
build/block-editor/style.css 10.9 kB 0 B
build/block-library/editor-rtl.css 7.23 kB 0 B
build/block-library/editor.css 7.24 kB 0 B
build/block-library/style-rtl.css 7.42 kB 0 B
build/block-library/style.css 7.43 kB 0 B
build/block-library/theme-rtl.css 669 B 0 B
build/block-library/theme.css 671 B 0 B
build/block-serialization-default-parser/index.js 1.65 kB 0 B
build/block-serialization-spec-parser/index.js 3.1 kB 0 B
build/blocks/index.js 57.6 kB 0 B
build/compose/index.js 6.21 kB 0 B
build/core-data/index.js 10.6 kB 0 B
build/data-controls/index.js 1.04 kB 0 B
build/data/index.js 8.2 kB 0 B
build/date/index.js 5.37 kB 0 B
build/deprecated/index.js 772 B 0 B
build/dom-ready/index.js 568 B 0 B
build/dom/index.js 3.06 kB 0 B
build/edit-post/index.js 91.2 kB 0 B
build/edit-post/style-rtl.css 8.52 kB 0 B
build/edit-post/style.css 8.51 kB 0 B
build/edit-site/index.js 5.07 kB 0 B
build/edit-site/style-rtl.css 2.53 kB 0 B
build/edit-site/style.css 2.53 kB 0 B
build/edit-widgets/index.js 4.43 kB 0 B
build/edit-widgets/style-rtl.css 2.58 kB 0 B
build/edit-widgets/style.css 2.58 kB 0 B
build/editor/editor-styles-rtl.css 381 B 0 B
build/editor/editor-styles.css 382 B 0 B
build/editor/style-rtl.css 3.97 kB 0 B
build/editor/style.css 3.96 kB 0 B
build/element/index.js 4.45 kB 0 B
build/escape-html/index.js 733 B 0 B
build/format-library/index.js 7.09 kB 0 B
build/format-library/style-rtl.css 502 B 0 B
build/format-library/style.css 502 B 0 B
build/hooks/index.js 1.93 kB 0 B
build/html-entities/index.js 621 B 0 B
build/i18n/index.js 3.49 kB 0 B
build/is-shallow-equal/index.js 710 B 0 B
build/keyboard-shortcuts/index.js 2.3 kB 0 B
build/keycodes/index.js 1.69 kB 0 B
build/list-reusable-blocks/index.js 2.99 kB 0 B
build/list-reusable-blocks/style-rtl.css 226 B 0 B
build/list-reusable-blocks/style.css 226 B 0 B
build/media-utils/index.js 4.84 kB 0 B
build/notices/index.js 1.58 kB 0 B
build/nux/index.js 3.01 kB 0 B
build/nux/style-rtl.css 616 B 0 B
build/nux/style.css 613 B 0 B
build/plugins/index.js 2.54 kB 0 B
build/primitives/index.js 1.5 kB 0 B
build/priority-queue/index.js 780 B 0 B
build/redux-routine/index.js 2.84 kB 0 B
build/rich-text/index.js 14.3 kB 0 B
build/server-side-render/index.js 2.55 kB 0 B
build/shortcode/index.js 1.7 kB 0 B
build/token-list/index.js 1.27 kB 0 B
build/url/index.js 4.01 kB 0 B
build/viewport/index.js 1.61 kB 0 B
build/warning/index.js 1.14 kB 0 B
build/wordcount/index.js 1.18 kB 0 B

compressed-size-action

`is_array` still warns if not set. Coincidentally helps avoid an unnecessary call to wp_get_registered_image_subsizes when there's no sizes to work with. One difference is that imageDimensions would not be set unless imageSizes is non-empty. But if the idea is for imageDimensions to be dependent upon imageSizes here, it's a sensible caveat.
Copy link
Contributor

@ryelle ryelle left a comment

Choose a reason for hiding this comment

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

Looks good - works with EditorsKit, tested with WP 5.4 RC2 & 5.3.2.

I'm not sure if the failing test is related, but otherwise 👍

@aduth
Copy link
Member Author

aduth commented Mar 17, 2020

I'm not sure if the failing test is related, but otherwise 👍

It's not one of the common intermittent failures, and seems like it ("Image block") is in the same realm of the changes here 🤔 I'll give it a look.

@aduth
Copy link
Member Author

aduth commented Mar 17, 2020

I wasn't able to reproduce the failure locally, and it passes after I restarted the build on Travis. I assume then it was just a flakey test.

@aduth aduth merged commit 014920f into master Mar 17, 2020
@aduth aduth deleted the fix/20907-image-dimensions-compat branch March 17, 2020 17:12
@github-actions github-actions bot added this to the Gutenberg 7.8 milestone Mar 17, 2020
@ellatrix ellatrix mentioned this pull request Jun 16, 2020
12 tasks
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 [Type] Bug An existing feature does not function as intended [Type] Plugin Interoperability Incompatibilities between a specific plugin and the block editor. Close with workaround notes.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Getting invalid argument error with 7.7 and 7.7.1
2 participants