Fix max width observer hook for align left/right images #69364
+31
−20
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
stokesman
added
[Type] Bug
stokesman
commented
Feb 28, 2025
Comment on lines
359
to
366
| // Only observe the max width from the parent container when the parent layout is not flex nor grid. | ||
| // This won't work for them because the container width changes with the image. | ||
| // TODO: Find a way to observe the container width for flex and grid layouts. | ||
| const isMaxWidthContainerWidth = | ||
| ! parentLayout || ! SIZED_LAYOUTS.includes( parentLayout.type ); | ||
| const [ maxWidthObserver, maxContentWidth ] = useMaxWidthObserver( | ||
| blockProps.className | ||
| ); |
There was a problem hiding this comment.
This is just moved down so the blockProps are available for passing in the className. I also changed the conditional to use the SIZED_LAYOUTS const introduced in #68666.
|
Size Change: +52 B (0%) Total Size: 1.84 MB
ℹ️ View Unchanged
|
stokesman
commented
Feb 28, 2025
stokesman
commented
Feb 28, 2025
Comment on lines
8
to
11
| const [ contentResizeListener, { width } ] = useResizeObserver(); | ||
| const observerRef = useRef(); | ||
| function useMaxWidthObserver( className ) { | ||
| const [ usedMaxWidth, setUsedMaxWidth ] = useState(); | ||
| const setResizeObserved = useResizeObserver( ( [ entry ] ) => { | ||
| setUsedMaxWidth( entry.contentRect.width ); | ||
| } ); |
There was a problem hiding this comment.
This is just using the new useResizeObserver API instead of the deprecated one.
stokesman
force-pushed
the
fix/max-width-observer-hook
branch
from
ce74c36 to
7a13c69
Compare
|
Flaky tests detected in 7a13c69. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/13591942245
|
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What?
In #63341 some logic was added to limit in-canvas resizing of images to be no wider than their container. It works quite well though there are some issues that sprang from it (like #69316 and #67506). Those issues are mostly avoided with #68666 but the underlying cause is still there and affects images that meet these conditions:
The underlying cause is that the max-width of an image can vary depending on its alignment yet the hook measures an element that remains the same size regardless of the image’s alignment.
Why?
The limitation to an image’s width should be accurate to what the theme actually specifies because it otherwise it won’t match the front end and breaks the WYSIWYG expectation. After #68666, the size in editor matches the front end but in the editor the resize handles are out of place.
How?
Adds the block’s alignment class names to the element that the hook measures to determine the max-width. Also, because that makes for a little more work that doesn’t always need to happen, this extends the hook with an
isActiveprop so it can return early when the element is not going to be added.Testing Instructions
Width limitation still works where it should
For unsized images aligned left or right, their width is limited or not depending on the active theme
Screenshots or screencast
TT5
max-width-observer-TT5-align-left-wrong.mp4
after-max-width-observer-TT5-align-left-okay.mp4
TT1
There’s actually no difference with this theme. It already works without this PR because the wrapper element added to align left/right images—through which the theme applies max-width styling to any child and that includes the element the hook measures.
max-width-observer-TT1-align-left-okay.mp4
after-max-width-observer-TT1-align-left-okay.mp4