WordPress.org

Make WordPress Core

Opened 5 years ago

Closed 5 years ago

#23649 closed defect (bug) (fixed)

Twenty Thirteen: Show portrait images correctly in gallery post formats

Reported by: obenland Owned by: 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 5 years ago.
currently.png (375.0 KB) - added by obenland 5 years ago.
after.png (410.8 KB) - added by obenland 5 years ago.
23649.1.diff (1.8 KB) - added by obenland 5 years ago.
23649.2.diff (1.2 KB) - added by obenland 5 years ago.

Download all attachments as: .zip

Change History (13)

@obenland
5 years ago

#1 @obenland
5 years ago

  • Description modified (diff)

#2 follow-up: @lancewillett
5 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
5 years ago

@obenland
5 years ago

#3 in reply to: ↑ 2 @obenland
5 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
5 years ago

#4 @lancewillett
5 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
5 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.

#6 @obenland
5 years ago

Related: #23695

@obenland
5 years ago

#7 @obenland
5 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
5 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.