Make WordPress Core

Opened 11 years ago

Closed 11 years ago

#23649 closed defect (bug) (fixed)

Twenty Thirteen: Show portrait images correctly in gallery post formats

Reported by: obenland's profile obenland Owned by: lancewillett's profile lancewillett
Milestone: 3.6 Priority: normal
Severity: normal Version: 3.6
Component: Bundled Theme Keywords: has-patch
Focuses: Cc:

Description (last modified by obenland)

The custom implementation of featured galleries in index view currently does not handle portrait formatted images very well:

http://screencast.com/t/NtKTpZdoa0M

Attachments (5)

23649.diff (1.7 KB) - added by obenland 11 years ago.
currently.png (375.0 KB) - added by obenland 11 years ago.
after.png (410.8 KB) - added by obenland 11 years ago.
23649.1.diff (1.8 KB) - added by obenland 11 years ago.
23649.2.diff (1.2 KB) - added by obenland 11 years ago.

Download all attachments as: .zip

Change History (13)

@obenland
11 years ago

#1 @obenland
11 years ago

  • Description modified (diff)

#2 follow-up: @lancewillett
11 years ago

Can you upload before / after screenshots?

In patch:

  1. max-height: 1000% looks unintentional :)
  2. For twentythirteen_get_attachment_link no other way to get that information from core and avoid this extra code? It'd be cool if core output the orientation for *all* themes to take advantage of it.

@obenland
11 years ago

@obenland
11 years ago

#3 in reply to: ↑ 2 @obenland
11 years ago

Replying to lancewillett:

  1. max-height: 1000% looks unintentional :)

It just needs to be set to a value where the image covers the whole width of the thumbnail. Or removed, which I will do in the next patch.


  1. For twentythirteen_get_attachment_link no other way to get that information from core and avoid this extra code? It'd be cool if core output the orientation for *all* themes to take advantage of it.

I'm not sure. wp_get_attachment_link() is a generic function that returns links to all attachment types, not only images. We could make that check in gallery_caption() and add a class to the icon tag. But I'm not sure it'll find its way into core in time.

@obenland
11 years ago

#4 @lancewillett
11 years ago

I'd vote "wontfix" to this ticket unless we can get the new class value into core -- it really should not be part of the theme.

If the layout can't be fixed without the class, let's redo the layout to center all the images and go away from the need for a "perfect grid" here.

#5 @lancewillett
11 years ago

Based on core office hours discussion today, obenland is going to work on a patch to add this to core gallery functionality, working with nacin to get it into 3.6 before freeze.

@obenland
11 years ago

#7 @obenland
11 years ago

Updated patch based on r23649, which also accounts for cases where the first image of a gallery happens to be in portrait orientation.

#8 @lancewillett
11 years ago

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

In 23652:

Twenty Thirteen: show portrait images correctly in gallery post formats. Props obenland, fixes #23649.

Note: See TracTickets for help on using tickets.