Remove image 100 percent width style. #28822
Conversation
|
Size Change: -6 B (0%) Total Size: 1.36 MB
ℹ️ View Unchanged
|
|
@tellthemachines and @jasmussen I am wondering if the image 100 percent width style is also influencing various issues: |
There was a problem hiding this comment.
Thanks so much for the PR. It's a simple change, those are always good. And the PR works well for simple cases:
However it breaks apart as soon as you use a small image, or a video. Here's a small image:
Notice how the resize handle floats in the middle. Here's before:
Notice how the photo is fuzzy because it's scaled beyond its intrinsic resolution, but at least the resize handle is accurate.
Here's a big video added:
It blows past the boundaries of the media area. Here's before:
Now in inspector testing, it did appear that at least the video issue could be solved by changing the max-width to 100% instead of "unset" — though if we did that, we would have to do a little digging to find out why it was set to "unset" in the first place (#9416).
Even if we did do that, though, we'd still have the issue for both images and video, where if the dimensions are too small for the allocated area, the resize handle will sit in the middle and do nothing until it reaches the image boundary:
So I see a few paths forward:
- The technical one, where we have to write a whole bunch of logic to figure out that the image is smaller than the allocated space, and then resize the media area accordingly.
- Apply the 100% max-width and decide that the resize handle floating in the middle is fine — that there isn't enough resolution so it shouldn't be scaled up.
- Decide that the current behavior is correct: that the Media & Text block is designed to crop and fit an image or video on the left, and that the separator is sole arbiter of the layout, and that if you add too small an image it's either because you want 8-bit graphics on the left, or you should either replace the image, or size the slider to the left to reach fidelity.
Honestly, 3 appeals the more to me. Even if the image is pixelated, there are tools to address that, and the block functions predictably. To an extent, it's the same with the cover block, or even the Group block once that gets background-image functionality: the fidelity of your design depends on the material you provide.
|
My gut feeling said that this might affect various blocks boundry area and the image inside it... |
|
Thanks for doing the testing @jasmussen, and I agree with your third suggestion - the block separator should determine the area where an image can be shown, and if the image is too small or of a different proportion to that area, it fills the empty space and gets cropped accordingly. If that makes the image look bad, it's up to the user to provide a proper image for it. That's basically what Facebook or Instagram etc do too. |
|
I'm putting in a vote for @jasmussen 's number 2 solution. My reasoning is:
|
|
I am wondering if this issue is associated: |
|
Pretty sure that's unrelated. This issues seems very specific to the |
|
Thanks for the detailed reviews and feedback here. I'm always keenly aware of the wide ranging side effects of changing even a single CSS line so I'm grateful to have the wider review. How do the solutions you describe apply to the issue reported in the original ticket? #17787 |
Going back to that one, the key issue I reviewed based on, was "a style is making my image too big". I initially thought the problem here was that a very small image when uploaded would become blurry when forcefully scaled up beyond its intrinsic resolution. However that might have been a misunderstanding on my part (focusing too much on the Removing the Right now the max-width is I created a separate PR with an alternate approach here: #28922 |
Excellent, thanks for continuing the work on this @jasmussen - your reasoning and solution seems more ideal. My change was based purely on the problem description in the issue and I am glad it spurred conversation leading to the real solution. I have one final question which I will leave on the new PR. Closing this one for now. |
Description
Fixes #17787
How has this been tested?
I also tested a video (uploaded mp4) and see a separate width 100% style.
Screenshots
Types of changes
Checklist: