-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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 image block centering when resizing to a smaller size #26376
Conversation
Size Change: +54 B (0%) Total Size: 1.21 MB
ℹ️ View Unchanged
|
This is an excellent PR. Thank you! Before: After: I would certainly agree that this PR enables the expected behavior, and that what's shipping is less ideal. However there's one problem, and it's the same problem that caused us to explore the It's still the case with floats, that's good: In fact, that may be good enough — if the block isn't floated, it is meant to take up the full width, right? But if we were to decide that the above is fine — a resized image that isn't floated can have a caption that spills beyond its own width — then we should likely solve this issue in a different way.
Curious on your thoughts, Kjell! Also CC: @jffng and @MichaelArestad in case you have thoughts. |
On a similar note to @jasmussen 's comment above, I notice that this solves the problem for the image, but leaves the caption centred: Before, both resized image and caption were left-aligned, as there was a bit of CSS that set the margin on resized figures to 0:
Could we just add that back, or was it breaking anything? |
@tellthemachines I think the alignment of the caption is theme specific (Twenty Twenty-One aligns it left). The caption should be centered from the styles the plugin provides by default, but as @jasmussen says it should also be the same width as the image. @jasmussen This does already seem broken in So it looks like the caption doesn't work anyway, at least not consistently. Possibly a separate bug? I'm not really sure how we can fix it without another wrapper around the image. |
To me that's an argument in favor of doing what you suggest — only limiting the caption width if an image is floated, or resized. However I still think if we agree on going that route, that it deserves looking into refactoring |
That screenshot was from Twenty Twenty One, so it's definitely broken then 😅 In any case, is there anything stopping us from doing what I suggested above?
|
Apologies, I forgot to respond to that one. IOU one milkshake 📝 This would only be in the editor, correct? In general I'm a little nervous both about touching this CSS at all, because it's so extremely widely used, but also because any additional CSS added is CSS that could affect how themes deal with it. Notably margins in a context where we have a centered column, can cause trouble when |
Milkshake accepted 😋
Yup, using We could still make it work by using |
Not really sure what a good solution is. I think the problem possibly originates from removing wrappers from the block. But also blocks in the editor have The columns block now also seems to have a very similar bug with a single column - #26538 |
Setting the margin to 0 should fix it as long as you don't set the width to 100%. This bug appeared because the margins on the table element were set to auto, so it gets centred. |
@tellthemachines I did remove the 100% width first.
|
You're both right.
This is because In other words, combinging That's why I think the solution outlined at the bottom of my previous comment is the way forward: only apply the |
It is impossible for me to keep with all the changes, is this related? WordPress/twentytwentyone#737 |
Looks unrelated, carolinan, but I'll respond on the TT1 repo. |
f4dcf32
to
990ba5a
Compare
@jasmussen I've made the change you suggested, let me know what you think. |
@jasmussen Good catch. I hadn't tested with a longer caption. The issue was that the editor handles alignments slightly differently, using a wrapper element with a data attribute, so I had to replicate the table styles for the editor. It should be working now with the last commit 😄 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the reviews, I think we got there eventually! |
* Fix image block centering when resizing to a smaller size * Revert previous 100% width fix * Remove display: table when image block is resized to avoid centering of block * Match frontend classes for alignment in editor
* Cover block: Restore default overlay background (#26569) * Restore default Cover overlay background The default Cover block overlay background was removed. This changed the style of existing blocks on existing sites. Restore the default background in such a way that it does not conflict with theme-provided background-color styles and there is no need to artificially add extra specificity. Fix regression: #26545 * Limit the interface skeleton to a max width of 100% to prevent some blocks managing to exceed the width (#26552) * Cover Block: Restore opacity controls (#26625) * Fix image block centering when resizing to a smaller size (#26376) * Fix image block centering when resizing to a smaller size * Revert previous 100% width fix * Remove display: table when image block is resized to avoid centering of block * Match frontend classes for alignment in editor * Gallery: Remove caption fallback for alt attribute (#26676) * Fix quote block default alignment (#26680) * Gallery: Remove obsolete deprecation entry (#26736) * Do not apply text color if it is being overriden (#24626) * Fix: Preset colors don't work on button block style outline (#26707) * Tests: Add fixture for Column deprecation (#26774) * Fix/column width units (#26757) * Fix issues with different units in column widths * Columns with fixed width should keep width on recalculation * Address review feedback * fix undefined index notice in Social Link Block (#25663) * fix undefined index notice * use isset instead of array_key_exists check * Update packages/block-library/src/social-link/index.php Co-authored-by: Greg Ziółkowski <grzegorz@gziolo.pl> Co-authored-by: Greg Ziółkowski <grzegorz@gziolo.pl> * Adds in missing styling for toolbar when using text-only setting (#26769) * Adds in missing styling for toolbar when using text-only setting * Increases margin * Moves style to correct file * Inserter: Fix 'Browse All' in Quick Inserter (#26443) * Inserter: Fix 'Browse All' in Quick Inserter Maintain the insertion point in @wordpress/block-editor state when moving from the Quick Inserter to the Inserter. This fixes a bug where the insertion point is wrong after clicking a block appender and selecting 'Browse All'. Care is taken to not regress a previous bug where the insertion point is wrong after clicking an inline insertion button and selecting 'Browse ALl'. * Inserter: Fix typo Co-authored-by: Daniel Richards <daniel.richards@automattic.com> * Call getBlockInsertionPoint once * Add useSelect dependencies * Make setInsertionPoint unstable * Avoid setting `isVisible` state when `SET_INSERTION_POINT` is triggered * Make index required and clarify rootClientId usage * Split insertionPoint into two reducers * Fix getInsertionPoint selector that expects default state of reducer to be null * Avoid resetting insertionPoint state on HIDE_INSERTION_POINT Co-authored-by: Daniel Richards <daniel.richards@automattic.com> Co-authored-by: Jon Surrell <jon.surrell@automattic.com> Co-authored-by: Jacopo Tomasone <Copons@users.noreply.github.com> Co-authored-by: Daniel Richards <daniel.richards@automattic.com> Co-authored-by: Bernie Reiter <ockham@raz.or.at> Co-authored-by: Greg Ziółkowski <grzegorz@gziolo.pl> Co-authored-by: Oliver Juhas <webmandesign@users.noreply.github.com> Co-authored-by: Jorge Costa <jorge.costa@automattic.com> Co-authored-by: Fabian Kägy <mail@fabian-kaegy.de> Co-authored-by: Tammie Lister <tammie@automattic.com> Co-authored-by: Robert Anderson <robert@noisysocks.com>
Description
Fixes #23986
When resizing an image to be smaller, the block was unexpectedly centering.
The wrapping
figure
for the image block is set todisplay: table
. When its contents are resized to a smaller size this wrapper's width collapses to its content size. At the same time blocks haveauto
left and right margin, so the image ends up centered.The fix I've applied is to add
with: 100%
to the figure so that it behave more likedisplay: block
.How has this been tested?
Types of changes
Bug fix (non-breaking change which fixes an issue)
Checklist: