Skip to content
This repository has been archived by the owner on Jan 15, 2019. It is now read-only.

post_thumbnail size assumes large images #208

Closed
joyously opened this issue Oct 20, 2018 · 7 comments
Closed

post_thumbnail size assumes large images #208

joyously opened this issue Oct 20, 2018 · 7 comments
Labels
bug Something isn't working

Comments

@joyously
Copy link

When viewing a post with a small featured image, the CSS causes the browser to upscale the image and it looks jagged and bad. It also takes up way too much space (it's a vertical image).
I think that the CSS should never use width:100% on images, because the CSS can't know what size they are.

@kjellr kjellr added the bug Something isn't working label Oct 20, 2018
@kjellr
Copy link
Collaborator

kjellr commented Oct 20, 2018

For visual reference, here's a screenshot using a tiny 80px by 60px featured image:

twentynineteen test__p 37

@nielslange
Copy link
Collaborator

@joyously & @kjellr On the other hand, having an 80x60 pixel featured image doesn’t look good either. Would it be a better approach to request a minimum height/width for the featured image?

@joyously
Copy link
Author

The one I had on my test site wasn't that small, but still it looked bad. And you can't enforce anything for legacy content. You could put a different class on it if it is below a threshold.
When I found this, I disabled the width:100% rule and the small image was repeating. It didn't look too bad that way...

@joyously
Copy link
Author

Should be in RC1 milestone.

@kjellr
Copy link
Collaborator

kjellr commented Nov 2, 2018

I did some exploration around this today, and did not arrive on a great solution to this issue. Here are some examples, each using one large image (2520×1415), one medium (640x480), and one small (160x160). I added some quick analysis based on my own personal impression of each option.

Option 1: Current

Large:
01-large

Medium:
01-medium

Small:
01-small

✅ Great: Large image looks ideal.
✅ Great: Medium image looks good, if not very slightly fuzzy.
🆗 Small image is pixellated, but fills the screen.

Option 2: Contain

Large:
02-large

Medium:
02-medium

Small:
02-small

✖️ Large image has borders. Appears broken.
✖️ Medium image has borders. Appears broken.
✖️ Small image is pixellated, has borders. Appears broken.

Option 3: Do Nothing

Large:
03-large

Medium:
03-medium

Small:
03-small

🆗 Large image is very zoomed in, missing details.
✖️ Medium image has a frame around it. Appears broken.
✖️ Small image is not pixellated, but has a frame around it. Appears broken.

Option 4: Repeat

This method is not compatible with #450.

Large:
04-large

Medium:
04-medium

Small:
04-small

🆗 Large image is very zoomed in, missing details.
✖️ Medium image almost fits, but doesn't quite. The repetition is confusing in this case.
🆗 Small image is not pixellated, but the repetition is more clear.


We could also technically target smaller images and give them a different treatment than larger images, but I worry that'd be confusing for users.

These results would vary depending on the types of images used and their subjects. That said, our current approach results in reasonable (or better) behavior when using the greatest variety of image sizes, so I'd prefer to continue with that at this point. I'm going to leave this issue out of RC1 for now, but if a PR arises with a good solution that works across all image sizes we'll happily fast-track it.

@joyously
Copy link
Author

joyously commented Nov 2, 2018

I think there is a bias here toward large images. Your assessment of the current implementation with the large image "Image looks ideal" is telling, because to me, that looks as bad as the rest. I think they all take up too much of the page, forcing the user to scroll to see any content.
Instead of trying to fit the image into what you think it should be, can't you just display the image, without trying to stretch across an entire window?

Here is an example of a site that would not work well with this design, because the images are tall and part of the content: http://www.moriareviews.com/ Obscuring the featured image with text and color, and not showing all of it because it's in the header defeats the purpose of featuring an image.

@kjellr
Copy link
Collaborator

kjellr commented Nov 26, 2018

Just wanted to note that I did explore a separate option to solve this problem by adding a toggle for the full screen featured images:

full-screen-image

However, that change relies on implementing a new feature. Now that we're in RC1, we aren't going to be undertaking any design tweaks of that size. That solution is archived here in case anyone wants to work with it in the future, but @allancole and I won't be incorporating it into the initial release.

The theme was designed for featured images to appear full-screen, like they are currently. So ruling the theme toggle out, we're already using the next best solution we have at this point (based on the explorations shared above at least), so I'm going to go ahead and close this issue for now. If a better solution arrives between now and release, we'll consider it, but the current behavior is working as designed.

@kjellr kjellr closed this as completed Nov 26, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants