WordPress.org

Make WordPress Core

Opened 7 years ago

Closed 7 years ago

Last modified 7 years ago

#26159 closed enhancement (fixed)

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

Reported by: silencerius Owned by: 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 7 years ago.
26159.diff (650 bytes) - added by shaunandrews 7 years ago.
26159.1.diff (431 bytes) - added by matveb 7 years ago.
26159.1.2.diff (431 bytes) - added by matveb 7 years ago.

Download all attachments as: .zip

Change History (21)

#1 @SergeyBiryukov
7 years ago

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

Duplicate of #26003.

#2 @matveb
7 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
7 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
7 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
7 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
7 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
7 years ago

  • Priority changed from normal to high

@shaunandrews
7 years ago

#8 @shaunandrews
7 years ago

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

#9 @matveb
7 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
7 years ago

  • Cc dreamwhisper added

#11 @nacin
7 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
7 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
7 years ago

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

Last edited 7 years ago by matveb (previous) (diff)

@matveb
7 years ago

@matveb
7 years ago

#14 @nacin
7 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
7 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
7 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
7 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.