Limit the max width of image to its container size #63341
Draft
+54
−32
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
kevin940726
added
[Type] Bug
|
Size Change: +9.16 kB (+0.52%) Total Size: 1.76 MB
ℹ️ View Unchanged
|
|
It seems close and I like the solution, but I think the main issue is that the image resized to the max bounds loses its responsive because it's set to an exact pixel value instead of 'auto'. I think it'd be better to set it back to 'auto' like an image is when first uploaded to an image block. Some steps to highlight the issue:
Now compare that behavior to this:
|
|
Another observation is that left/right aligned images can no longer be resized to the same degree they were before (it seems to be limited to content width). That's an interesting one, I'm not sure what the preferred behavior is. This is also a little bit theme dependent, some themes like TT4 show aligned blocks offset from the content which would allow them to be resized further, while others don't. That makes it tricky to know dynamically what the right limit is. |
mtias
added
the
[Feature] Media
|
Nice work. Just chiming in to say that I tested this and it fixes the issue 😀 |
+1, nice progress here, it feels much better to use to me with the constrained maximum.
Great idea, that sounds like a decent way to handle it. Would that be a matter of setting to Also just pinging @Mamaduka for visibility on this one since he commented on the linked issue 🙂 |
|
Nice. I've not thought of using |
I've been thinking about this. I think it's hard to distinguish the user's intention just by observing resizing. Maybe the user wants it to be a certain exact pixel wide. I think in most cases the user wants it to fit the max width though 😅. I wonder if some kind of "snapping" makes it clearer. Perhaps @WordPress/gutenberg-design has some ideas? 🙇
Another thing to note is that I think it also depends on the original image's size. If the image width is smaller than the max width, then resizing it to the max width (the user intention) should probably mean "width: 100%" rather than "width: auto"? 🤔 |
Good point. In this case, if folks want to set an exact pixel size, they can still do that in the sidebar. I like the idea of some kind of snapping at the max width that sets it to
Also a good point! The idea of defaulting to |
Possibly, I haven't checked the CSS. When I say 'auto', it's what the UI shows in the width field. Sorry if that wasn't clear. |
I'd echo fixed width vs. "unbounded" is one of the primary headaches to solve as part of this. And if we can do snapping to unbounded ("Auto" as Dan refers to), that would be great. We've had separate explorations into snapping into wide and full-wide alignments too, it's very difficult to build. But it would definitely be valuable, and in this case would account for snapping to content-width. |
|
I found this old issue that might be closed by this PR - #12168. |
|
Should we include the snapping feature in this PR, or could it be implemented separately? Is the implementation in this PR without snapping worth merging? |
|
Is there a basic version that could be attempted? Something like this pseudocode onResizeStop( { actualWidth } ) {
if ( Math.abs( containerWidth - actualWidth ) < 10 ) {
setWidth( '100%' );
} else {
setWidth( actualWidth )
}
} |
Committed in 858d8a2! |
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?
Attempt to fix #63326.
Limit the max width of image to its container size.
Why?
See #63326. Images in columns or other container can be resized to be bigger than its container, which is not expected.
How?
Limit the
maxWidthof the image to its container's size. Not sure about this approach and potential side effects though.Testing Instructions
Screenshots or screencast
Kapture.2024-07-10.at.15.54.58.mp4