Make WordPress Core

Opened 8 years ago

Closed 3 months ago

#39253 closed defect (bug) (fixed)

Twenty Seventeen: Head Image Quality Issue

Reported by: richardevs's profile richardevs Owned by: karmatosed's profile karmatosed
Milestone: 6.6 Priority: normal
Severity: normal Version: 4.8
Component: Bundled Theme Keywords: needs-testing has-patch commit
Focuses: css Cc:

Description

A question about the Twenty Seventeen Theme's Head Image, it is really cool but the image is not in the best position on iOS Safari and image quality drops too...

https://holywhite.com/wp-content/uploads/2016/12/Evernote-Camera-Roll-00281213-053634.png

But if I turn the phone around...

https://holywhite.com/wp-content/uploads/2016/12/Evernote-Camera-Roll-00281213-053635.png

Looking much better, refresh it and rotate again?

https://holywhite.com/wp-content/uploads/2016/12/Evernote-Camera-Roll-00281213-053636.png

Wow the shiny picture comes back...

P.S. Is it possible to set the align of Head Image on mobile? So I can choose which part of the picture I mostly wanna show.

Attachments (1)

55561.diff (919 bytes) - added by sabernhardt 3 months ago.
increases size recommendation to 200vw for screens up to 767px wide

Download all attachments as: .zip

Change History (13)

#1 @laurelfulford
8 years ago

Good catch, @richardevs!

It looks like the image quality issue is happening because the header image file itself is set to be 100% of the screen width on page load:

function twentyseventeen_header_image_tag( $html, $header, $attr ) {
	if ( isset( $attr['sizes'] ) ) {
		$html = str_replace( $attr['sizes'], '100vw', $html );
	}
	return $html;
}
add_filter( 'get_header_image_tag', 'twentyseventeen_header_image_tag', 10, 3 );

... and then it's being scaled up further with object-fit: cover in the style.css. It's much more noticeable on vertical screens, because the image is getting scaled up a lot more than on a horizontal screen to fit the space.

I'm not sure if the best fix is to simply increase the image size for smaller screens, or perhaps there's some really specific case we can check for before assigning a size?

#2 @davidakennedy
8 years ago

Thanks for taking a look @laurelfulford, and providing more details! Wondering if @joemcgill has thoughts on the best approach to this one?

#3 @joemcgill
8 years ago

This is a tricky one. If there's a way to calculate the actual image width that will be applied using object: cover at different break points, then we could update the sizes attribute accordingly. It's worth noting that calc() works inside sizes attributes. If that doesn't work, then a possible approach could be to estimate when the image will be larger than 100% of the screen size and do something like 150vw.

#4 @SergeyBiryukov
8 years ago

  • Summary changed from Head Image Quality Issue to Twenty Seventeen: Head Image Quality Issue

#5 @davidakennedy
8 years ago

  • Keywords needs-patch added
  • Milestone changed from Awaiting Review to Future Release

#6 @ianbelanger
4 years ago

  • Focuses css added
  • Keywords needs-testing 2nd-opinion added

Can someone test this again on an IOS device? I would like to see if the issue still exists.

#7 @sabernhardt
3 months ago

#55561 was marked as a duplicate. (props shailu25 and robertghetau)

Last edited 3 months ago by sabernhardt (previous) (diff)

@sabernhardt
3 months ago

increases size recommendation to 200vw for screens up to 767px wide

#8 @sabernhardt
3 months ago

  • Keywords has-patch added; needs-patch 2nd-opinion removed
  • Milestone changed from Future Release to 6.6

Yes, the blurriness/pixelation can still occur.

I moved 55561.diff from the other ticket and kept the same filename. Also, shailu25 added a test report there: ticket:55561#comment:6.

I chose 200vw for the smaller screens so it would be less than the full size. Increasing further to 2000px might avoid blurriness at any height, though the image probably would be larger than necessary.

The image fills a container that is 75vh tall, which would make the image width 125vh at a ratio of 5:3 (but vh does not seem to work in the sizes attribute).

#9 @poena
3 months ago

Tested on Windows 11, Chrome, using the landscape image from 55561. There is a visible quality improvement on smaller screen widths.

#10 @karmatosed
3 months ago

Based on there being a visible quality improvement on smaller screens I would recommend this for commit. Thank you everyone.

#11 @karmatosed
3 months ago

  • Keywords commit added

#12 @karmatosed
3 months ago

  • Owner set to karmatosed
  • Resolution set to fixed
  • Status changed from new to closed

In 58135:

Twenty Seventeen: Resolves Header Image Quality Issue

The heading image had quality issues on iOS Safari in portrait and landscape modes. This resolves it through adding 200cv for smaller screens so less than full size.

Props poena, richardevs, laurelfulford, davidakennedy, joemcgill, SergeyBiryukov, ianbelanger, sabernhardt, shailu25, robertghetau.
Fixes #39253.

Note: See TracTickets for help on using tickets.