Make WordPress Core

Opened 11 years ago

Closed 11 years ago

Last modified 11 years ago

#26159 closed enhancement (fixed)

Theme screenshots are being cut off in the theme selection screen in 3.8

Reported by: silencerius's profile silencerius Owned by: nacin's profile nacin
Milestone: 3.8 Priority: high
Severity: normal Version: 3.8
Component: Customize Keywords: has-patch
Focuses: Cc:

Description

About 50px of the bottom parts of themes' screenshot.png files are hidden from the Theme selector screen in /wp-admin/themes.

This can cause issues to theme vendors, see attached screenshot.

Attachments (4)

screenshots_hidden.png (224.8 KB) - added by silencerius 11 years ago.
26159.diff (650 bytes) - added by shaunandrews 11 years ago.
26159.1.diff (431 bytes) - added by matveb 11 years ago.
26159.1.2.diff (431 bytes) - added by matveb 11 years ago.

Download all attachments as: .zip

Change History (21)

#1 @SergeyBiryukov
11 years ago

  • Milestone Awaiting Review deleted
  • Resolution set to duplicate
  • Status changed from new to closed

Duplicate of #26003.

#2 @matveb
11 years ago

This is different from #26003:

We have changed the screenshot aspect ratio for the theme grid to 1.5:1, among other things to keep size sane as we are bumping the dimension guidelines to 440px width (880 on retina). We'll post this for theme authors as soon as we can. The way it's implemented is so as to not distort old screenshots under the new constrains.

#3 follow-up: @dd32
11 years ago

  • Milestone set to 3.8
  • Resolution duplicate deleted
  • Status changed from closed to reopened

I'm re-opening this for now to reconsider the suggested theme screenshots..

This really feels like we're changing something and making all existing screenshots "invalid" for no particularly good reason. Most themes are also include more vertical size rather than horizontal, so wide-screen thumbnails are not really appropriate for a theme.

Has there been any discussion on if this is really the best route for us to go?

#4 @matveb
11 years ago

This was discussed quite a bit during the design phase. Screenshots won't really become invalid, we just changed how we display them on the grid for browsing purposes. Also, wide screenshots mimic screen layouts better and that's something we wanted to convey. Regardless, the detailed view can have the 4:3 aspect ratio showing the full screenshot, so nothing is lost, just adapted for the different contexts.

#5 in reply to: ↑ 3 @shaunandrews
11 years ago

Replying to dd32:

This really feels like we're changing something and making all existing screenshots "invalid" for no particularly good reason.

There are a few reasons for this change:

  • We're showing larger images (and retina images), so making the default smaller means a smaller page load. Now, to keep things easy, we're loading the taller screenshots and hiding the bottom portion. This means that the page load improvements won't be seen until (unless) theme authors update their screenshots.
  • Shorter screenshots means more themes will show within the vertically available space in your window, which should (hopefully) let you browse quicker.
  • The detail modal shows the full-size screenshot, and (I believe) has support for multiple screenshots. We have plans/ideas for future iterations to make viewing screenshots even better in the modal.

Most themes are also include more vertical size rather than horizontal, so wide-screen thumbnails are not really appropriate for a theme.

Its impossible to say if widescreen is better than portrait, as browser windows and screen sizes vary from user-to-user and device-to-device. A taller screenshot is not more representative of a theme than a wider screenshot — however wider formats are better for browsing. The detail modal is where the screenshots can really shine.

I understand that the example that silencerius provides includes some branding elements in the screenshot, which are now obscured in the new design. I hope the incentive to update these screenshots (higher resolution and support for more screenshots) balances the need to redo your screenshot to move the branding elements. Though, in my opinion, the screenshot should be just that — a screenshot — and not a place for logos, theme names, company names, or other branding elements.

#6 @nacin
11 years ago

Has there been any discussion on if this is really the best route for us to go?

There was no public discussion that I saw. That includes a lack of discussion with the theme reviewers or other theme authors. Unfortunately we can't just make unilateral decisions on something this major. Please correct me if I missed a thread somewhere.

WordPress.com can do whatever it wants, but I'm going to suggest we modify the design in core to work with existing screenshot ratios.

#7 @nacin
11 years ago

  • Priority changed from normal to high

@shaunandrews
11 years ago

#8 @shaunandrews
11 years ago

The above diff changes the ratio back to 1.33 — screenshots are no longer truncated.

#9 @matveb
11 years ago

Just to be clear, at this stage we are not proposing a change in ratio to the guidelines. At most, we would display themes at 1.5/1 on the grid design, and the full 4/3 on the detail view.

Now, we do need to bump the size a bit to avoid blurriness. 450px seems like it would cover our scenarios and media query break points pretty well, without going overboard. It can also be lower: between 420 and 450 covers our uses very well. (Keeping retina in mind, that's 840-900 we are talking about.)

Themes with old-retina sizes will be fine for the most part, except they will look blurry in some cases — specially if they weren't retina before. There's not much we can do about that except perhaps show the native resolution with black bars surrounding it, which I don't think is worthy nor very elegant.

I want to start doing this with virtually zero impact to themes, at least until we tackle install. This should be absolutely compatible with old screenshots while allowing those who want to take the full benefit of the new UI.

I don't think we need to revert the truncated display on the grid view. We will be displaying the full screenshot on the detailed view, and the cropping will be done on our side, just presentational.

So, what we need to discuss is the width limit — what do you think about the 450px width (900 hi-dpi)?

#10 @dreamwhisper
11 years ago

  • Cc dreamwhisper added

#11 @nacin
11 years ago

  • Component changed from Themes to Appearance
  • Keywords has-patch added

So is 26159.diff good? Do we need more than that?

#12 @shaunandrews
11 years ago

I'm cool with that diff. We can update the guidelines (with a post on make/core, perhaps) and push for the changes in 3.9.

#13 @matveb
11 years ago

I'm making a patch that restores full aspect ration on overlay but keeps cropped versions on grid.

Version 0, edited 11 years ago by matveb (next)

@matveb
11 years ago

@matveb
11 years ago

#14 @nacin
11 years ago

  • Owner set to nacin
  • Resolution set to fixed
  • Status changed from reopened to closed

In 26739:

Themes: Adjust screenshot ratios.

  • On the details overlay, always show the full 4/3 ratio.
  • On the grid, reduce to ~3/2 only if the screen width is less than 1400px. Otherwise, show the screenshot at 4/3 ratio.

props matveb, nacin.
fixes #26159.

#15 follow-up: @nacin
11 years ago

3/2 is still the recommended screenshot ratio. It used to be 300x225, or 600x450 to support HiDPI. That's changing to 440x330, or 880x660 to support HiDPI. More to come on the theme-reviewers mailing list and probably on http://make.wordpress.org/themes.

The screenshot is still going to get cut-off a bit on themes.php, for look-and-feel reasons. At bigger breakpoints (1640px and up) it won't get cut off. See #26462.

#16 in reply to: ↑ 15 @matveb
11 years ago

Replying to nacin:

The screenshot is still going to get cut-off a bit on themes.php, for look-and-feel reasons. At bigger breakpoints (1640px and up) it won't get cut off. See #26462.

Expanding on that, the detailed view on themes.php for each theme will display the 4/3 version at all screen sizes.

#17 @lancewillett
11 years ago

In 26749:

Bundled themes: new screenshots at 880x660 to maximize theme goodness in the new theme viewer design. See #26159.

Note: See TracTickets for help on using tickets.