WordPress.org

Make WordPress Core

Opened 8 years ago

Closed 8 years ago

#25524 closed defect (bug) (wontfix)

Twenty Thirteen – single attachment page image height

Reported by: Marco_Teethgrinder Owned by:
Milestone: Priority: low
Severity: normal Version: 3.6
Component: Bundled Theme Keywords: has-patch
Focuses: Cc:

Description

In image attachment pages the height of the image gets reduced to a max of 724px, even though it could fit in full size with no problem whatsoever.

Proposed fix in functions.php line 412

from
$attachment_size = apply_filters( 'twentythirteen_attachment_size', array( 724, 724 ) );
to
$attachment_size = apply_filters( 'twentythirteen_attachment_size', array( 724, 9999 ) );

Attachments (1)

25524.diff (620 bytes) - added by slobodanmanic 8 years ago.
Updated attachment height value

Download all attachments as: .zip

Change History (9)

@slobodanmanic
8 years ago

Updated attachment height value

#1 @slobodanmanic
8 years ago

  • Cc slobodan.manic@… added

#2 @obenland
8 years ago

  • Keywords has-patch added
  • Milestone changed from Awaiting Review to 3.7
  • Version changed from trunk to 3.6

This is a bug and should be fixed.

@lancewillett, if you're not happy with 9999 as an arbitrary number, we could use 1288, which would allow images up to the ration of 16:9 to be displayed in full width.

#3 follow-ups: @lancewillett
8 years ago

Turns out it's exactly the opposite. We explicitly decided not to use a big number like 9999 because of portrait images. If you have a really big portrait image it takes up way too much room on the page, and also is jarring when viewing a set of images in a row.

Setting the height to 724 means it could fit nicely in the same aspect ratio as landscape images, creating a nice flow to the image sizes.

#4 in reply to: ↑ 3 @obenland
8 years ago

Replying to lancewillett:

Setting the height to 724 means it could fit nicely in the same aspect ratio as landscape images, creating a nice flow to the image sizes.

In the example they showed me it looked really bad. Maybe because it was 16:9? It looks good on the demo page though. Hm.

#5 @lancewillett
8 years ago

  • Milestone changed from 3.7 to 3.8
  • Priority changed from normal to low

Low priority, can re-look at this closer to 3.8.

#6 @RDall
8 years ago

Just wanted to add my support to getting this fixed in the 3.8

#7 in reply to: ↑ 3 ; follow-up: @chipbennett
8 years ago

Replying to lancewillett:

Turns out it's exactly the opposite. We explicitly decided not to use a big number like 9999 because of portrait images. If you have a really big portrait image it takes up way too much room on the page, and also is jarring when viewing a set of images in a row.

Is twentythirteen_the_attached_image() only output in image.php? If so, you should never have a case of "viewing a set of images in a row", since image.php is a single-post view for an attachment image.

Setting the height to 724 means it could fit nicely in the same aspect ratio as landscape images, creating a nice flow to the image sizes.

Again: there will only ever be one image output at $attachment_size in image.php, so there will be no other images for the attachment image to "flow" with.

#8 in reply to: ↑ 7 @obenland
8 years ago

  • Milestone 3.8 deleted
  • Resolution set to wontfix
  • Status changed from new to closed

Replying to chipbennett:

Is twentythirteen_the_attached_image() only output in image.php? If so, you should never have a case of "viewing a set of images in a row", since image.php is a single-post view for an attachment image.

Yes and no. twentythirteen_the_attached_image() links the current image to the next, to achieve somewhat of a gallery function on image pages. This way you can navigate between images attached to the same post, without having to go back to its parent every time. Hence the mention of flow.

Anyway, closing this one. Too minor of an issue to get our hands dirty with backwards compatibility and such.

Note: See TracTickets for help on using tickets.