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.

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

Jump to bottom

Media & Text Block: Fix the "Crop image to fill entire column" option #20439

Merged
merged 2 commits into from
Feb 26, 2020
Merged

Media & Text Block: Fix the "Crop image to fill entire column" option #20439

merged 2 commits into from
Feb 26, 2020

Conversation

WunderBart
Copy link
Member

@WunderBart WunderBart commented Feb 25, 2020 •
edited
Loading

Description

Closes: #20434

How has this been tested?

  1. Check-out this branch,
  2. Add Media & Text block to any page,
  3. Add an image (left block column),
  4. Add enough text to exceed the image height (right block column),
  5. Turn on the Crop image to fill entire column option in the block settings panel on the right,
  6. The image should crop & fill the whole column,
  7. Focal Point Picker option should work as expected.

Screenshots

Before the fix:

Screenshot 2020-02-25 16 08 16

After the fix, image fills the column:

Screenshot 2020-02-25 16 08 53

After the fix, Focal Point Picker works as expected:

Screenshot 2020-02-25 16 09 19

After the fix, page view:

image

Types of changes

Bug fix (non-breaking change which fixes an issue)

Checklist:

  • My code follows the WordPress code style.

@WunderBart WunderBart added [Type] Bug An existing feature does not function as intended [Block] Media & Text Affects the Media & Text Block labels Feb 25, 2020
@WunderBart WunderBart self-assigned this Feb 25, 2020
@github-actions
Copy link

Size Change: -1 B

Total Size: 864 kB

Filename Size Change
build/block-library/editor-rtl.css 7.67 kB -3 B (0%)
build/block-library/editor.css 7.67 kB -2 B (0%)
build/block-library/index.js 116 kB -1 B
build/block-library/style-rtl.css 7.49 kB +2 B (0%)
build/block-library/style.css 7.5 kB +3 B (0%)
ℹ️ View Unchanged
Filename Size Change
build/a11y/index.js 1 kB 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/index.js 104 kB 0 B
build/block-editor/style-rtl.css 10.3 kB 0 B
build/block-editor/style.css 10.3 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/components/index.js 190 kB 0 B
build/components/style-rtl.css 15.5 kB 0 B
build/components/style.css 15.5 kB 0 B
build/compose/index.js 5.76 kB 0 B
build/core-data/index.js 10.5 kB 0 B
build/data-controls/index.js 1.03 kB 0 B
build/data/index.js 8.22 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 90.9 kB 0 B
build/edit-post/style-rtl.css 8.59 kB 0 B
build/edit-post/style.css 8.58 kB 0 B
build/edit-site/index.js 4.59 kB 0 B
build/edit-site/style-rtl.css 2.77 kB 0 B
build/edit-site/style.css 2.76 kB 0 B
build/edit-widgets/index.js 4.43 kB 0 B
build/edit-widgets/style-rtl.css 2.59 kB 0 B
build/edit-widgets/style.css 2.58 kB 0 B
build/editor/editor-styles-rtl.css 325 B 0 B
build/editor/editor-styles.css 327 B 0 B
build/editor/index.js 44.6 kB 0 B
build/editor/style-rtl.css 4.01 kB 0 B
build/editor/style.css 4 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.6 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.92 kB 0 B
build/html-entities/index.js 622 B 0 B
build/i18n/index.js 3.45 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.68 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.85 kB 0 B
build/notices/index.js 1.57 kB 0 B
build/nux/index.js 3.02 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.49 kB 0 B
build/priority-queue/index.js 879 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.54 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 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

@WunderBart WunderBart mentioned this pull request Feb 25, 2020
Closed
@obenland
Copy link
Member

Is there any information as to what caused the bug?

@WunderBart
Copy link
Member Author

WunderBart commented Feb 25, 2020 •
edited
Loading

Is there any information as to what caused the bug?

@obenland, I believe it was introduced in #18347. You can see there, that the class name used for the figure element didn't match the one used in the markup (applied to the figure element here).

obenland
obenland approved these changes Feb 25, 2020
View reviewed changes
Copy link
Member

@obenland obenland left a comment

Choose a reason for hiding this comment

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

Tested it with vertical and horizontal images and it works as expected now. I also made sure that the bug that was previously fixed is still fixed.

@WunderBart WunderBart merged commit 5dab5f7 into master Feb 26, 2020
@WunderBart WunderBart deleted the fix/media-text-block-image-fill branch February 26, 2020 12:05
@github-actions github-actions bot added this to the Gutenberg 7.7 milestone Feb 26, 2020
@jffng jffng mentioned this pull request Feb 28, 2020
Merged
@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 Mar 3, 2020
@jorgefilipecosta jorgefilipecosta mentioned this pull request Mar 9, 2020
Closed
jorgefilipecosta pushed a commit that referenced this pull request Mar 9, 2020
@WunderBart @jorgefilipecosta
Media & Text Block: Fix the "Crop image to fill entire column" option (…
876e32d 
…#20439)

* Use correct class name for media container element

* Rename media container class name for consistency
@jorgefilipecosta jorgefilipecosta mentioned this pull request Mar 9, 2020
Merged
jorgefilipecosta pushed a commit that referenced this pull request Mar 10, 2020
@WunderBart @jorgefilipecosta
Media & Text Block: Fix the "Crop image to fill entire column" option (…
ece56ae 
…#20439)

* Use correct class name for media container element

* Rename media container class name for consistency
@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 Mar 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@obenland obenland obenland approved these changes

@ajitbohra ajitbohra Awaiting requested review from ajitbohra

@jorgefilipecosta jorgefilipecosta Awaiting requested review from jorgefilipecosta

@Soean Soean Awaiting requested review from Soean

@talldan talldan Awaiting requested review from talldan

@frontdevde frontdevde Awaiting requested review from frontdevde

Assignees

@WunderBart WunderBart

Labels
[Block] Media & Text Affects the Media & Text Block [Type] Bug An existing feature does not function as intended
Projects
None yet
Milestone
Gutenberg 7.7
Development

Successfully merging this pull request may close these issues.

Media & Text Block: image crop & fill option is broken
3 participants
@WunderBart @obenland @jorgefilipecosta