Make WordPress Core

Opened 12 years ago

Closed 10 years ago

#19816 closed enhancement (duplicate)

Theme details should be able to include multiple screenshots

Reported by: helen's profile helen Owned by:
Milestone: Priority: normal
Severity: normal Version: 3.3
Component: Themes Keywords:
Focuses: Cc:

Description

Per 3.4 scoping, themes should be able show multiple screenshots. This would require an API change on .org.

Ideas include showing a gallery inline with the current placement of the screenshot. Other visual ideas welcome. The thickbox will likely disappear and would not serve as a good place, as it is used for installing and not shown for installed themes.

Attachments (11)

radio-button-slider-or-slideshow.jpg (146.8 KB) - added by brandondove 12 years ago.
Using radio buttons to toggle through the screenshots.
arrows-slider-or-slideshow.jpg (146.7 KB) - added by brandondove 12 years ago.
Using arrows to toggle through the screenshots with a text indicator showing how many screenshots there are total and what image the user is looking at..
stack.png (2.0 KB) - added by helenyhou 12 years ago.
Stack image, to go in wp-admin/images
19816.diff (4.2 KB) - added by helenyhou 12 years ago.
First rough pass
19816.2.diff (9.1 KB) - added by helenyhou 12 years ago.
19816.3.diff (10.0 KB) - added by kirasong 12 years ago.
Fixed Whitespace issue on expansion, clearing of details on resize, caching of $(this), code
19816.3.2.diff (10.0 KB) - added by kirasong 12 years ago.
Fixed whitespace issue on expansion of details div, clearing of details on window resize, caching of $(this), whitespace for coding standards. [edit: trailing space removal]
19816.4.diff (10.9 KB) - added by helenyhou 12 years ago.
19816.4.2.diff (10.9 KB) - added by kirasong 12 years ago.
19816.4.diff + switching to .data() for grabbing number of screenshots.
19816.5.diff (10.7 KB) - added by helenyhou 12 years ago.
19816.7.diff (12.1 KB) - added by helenyhou 12 years ago.

Download all attachments as: .zip

Change History (44)

#1 @kirasong
12 years ago

  • Cc mike.schroder@… added

#2 @azizur
12 years ago

  • Cc azizur added

#3 @DrewAPicture
12 years ago

  • Cc xoodrew@… added

#4 @brandondove
12 years ago

  • Cc brandon@… added

@brandondove
12 years ago

Using radio buttons to toggle through the screenshots.

@brandondove
12 years ago

Using arrows to toggle through the screenshots with a text indicator showing how many screenshots there are total and what image the user is looking at..

#5 follow-up: @brandondove
12 years ago

Those icons next to the links in my mocks should probably be aligned a bit more with standard WordPress icons. They were more for placement and style than actual usage.

#6 @kwight
12 years ago

  • Cc kwight@… added

#7 @ramiy
12 years ago

  • Cc r_a_m_i@… added

#8 in reply to: ↑ 5 @helenyhou
12 years ago

Replying to brandondove:

Those icons next to the links in my mocks should probably be aligned a bit more with standard WordPress icons. They were more for placement and style than actual usage.

From Jane's comment on Make UI: No slider, please. Also, I don't think we're talking about adding icons here.

#9 @cais
12 years ago

  • Cc edward.caissie@… added

#10 @helenyhou
12 years ago

A note about screenshot sizes: the plan is to stick to the same size that appears in the install thickbox for the multiple screenshots - 300px by 225px. Also going to up the size of the single screenshot that shows in the list table view to match. Have run this by the Theme Review Team and don't foresee any issues.

@helenyhou
12 years ago

Stack image, to go in wp-admin/images

@helenyhou
12 years ago

First rough pass

#11 @helenyhou
12 years ago

19816.diff is a rough initial pass with heavily commented JS. JS used to determine where to pop in the extended details div. Currently looks like this: http://cl.ly/3h3Q1Y290N3G083J1L42 - just scraped the other screenshots in for demonstration, and the stack would only show for themes with multiple screenshots. The stack does intentionally disappear when the layer is open. Thinking perhaps the screenshot grid within the extended details div should match that of the list table.

Things to work on:

  • Adding .has-screenshots to .available-theme divs when applicable
  • Retrieving the multiple screenshots in get_themes() and adding to the .org API response
  • Appending the images into the extended details div
  • Reflow on window resize
  • Star rating image has a colored (white) background by necessity and therefore doesn't match
  • CSS cleanup (colors)

Tested down to IE8 so far. No JS view remains the same as before.

@helenyhou
12 years ago

#12 @helenyhou
12 years ago

19816.2.diff adds in .has-screenshots (needs stack.png in wp-admin/images, not added in the patch) and handles screenshots as an array, as well as appending them in to the extended details div.

Thinking further, we could probably just have both get_themes() and the .org API response continue to return the first screenshot, as it does now, and then just the number of screenshots. The full URL for all of the screenshots isn't needed, since naming and file extension (.png) should be consistent for additional images. The JS in this patch only receives the total number of images and builds the image src URL from there.

@kirasong
12 years ago

Fixed Whitespace issue on expansion, clearing of details on resize, caching of $(this), code

#13 @kirasong
12 years ago

Added 19816.3.2.diff.
Fixed whitespace issue on expansion of details div, clearing of details on window resize, caching of $(this), whitespace for coding standards.

Last edited 12 years ago by helenyhou (previous) (diff)

@kirasong
12 years ago

Fixed whitespace issue on expansion of details div, clearing of details on window resize, caching of $(this), whitespace for coding standards. [edit: trailing space removal]

#14 @nacin
12 years ago

In [20029]:

Introduce WP_Theme, wp_get_themes(), and wp_get_theme() to replace get_themes(), get_theme(), get_theme_data(), current_theme_info(), and others.

  • Getters and Helpers: Introduces a series of methods to allow for easy generation of headers for display, and other theme metadata, including page templates.
  • Screenshots: Handles support for multiple screenshots. (see # Additional screenshots must be PNG and start with screenshot-2.png, and be sequential to be counted. see #19816.
  • Error Handling: Broken themes have a WP_Error object attached to them.
  • Caching: Introduces a wp_cache_themes_persistently filter (also in [20020]) to enable persistent caching of all filesystem and sanitization operations normally handled by WP_Theme (and formerly get_file_data() and get_themes()). Themes are cached individually and across five different cache keys for different data pieces.
  • Compatibility: A WP_Theme object is backwards compatible with a theme's array formerly returned by get_themes() and get_theme(), and an stdClass object formerly returned by current_theme_info().
  • i18n/L10n: Theme headers are now localizable with proper Text Domain and Domain Path headers, like plugins. (Language packs may remove the requirement for headers.) For page templates, see #6007 (not fixed yet, but will be easy now). For headers, fixes #15858.
  • PHP and CSS files: New methods that fetch a list of theme files (for the theme editor) only on demand, rather than only loading them into memory. fixes #11214.

Functions deprecated:

  • get_themes(), get_allowed_themes() and get_broken_themes() -- use wp_get_themes()
  • get_theme() and current_theme_info() -- use wp_get_theme()
  • get_site_allowed_themes() -- use WP_Theme::get_allowed_on_network()
  • wpmu_get_blog_allowedthemes() -- use WP_theme::get_allowed_on_site()

see also [20016], [20018], [20019], [20020], [20021], [20022], [20025], [20026], [20027]. also fixes #19244.

see #20103.

@helenyhou
12 years ago

#15 @helenyhou
12 years ago

19816.4.diff is a refresh to use the beautiful WP_Theme for the screenshot count. Theme install side will likely need to be tweaked later to accommodate whatever gets returned from the .org API, and then should probably have variable names actually match. Would like some eyes on the JS and then to get this in for a look at UX. It all works nicely - add some appropriately named screenshots to an installed theme folder for a look. And stack.png.

One note about the UX that started with 19816.3.diff - the details div snaps shut when the window is resized. We can look at other behaviors, but this seemed the most sane for a first go. Nothing particularly breaks on window resize, but there is some funkiness without the function on $(window).resize().

@kirasong
12 years ago

19816.4.diff + switching to .data() for grabbing number of screenshots.

#16 @kirasong
12 years ago

Added 19816.4.2.diff:
19816.4.diff + switching from .attr() to .data() for grabbing number of screenshots and a few coding standards/comment changes.

#17 @nacin
12 years ago

In [20043]:

Have WP_Theme::get_screenshot() default to an absolute URI. Allow 'relative' to be requested. see #20103, see #19816.

#18 follow-ups: @greenshady
12 years ago

Will the extra theme screenshots (other than screenshot.png) be housed in the top level of the theme directory like we do with plugins? Or, will there be a /screenshots folder, or at least an option to filter it to do so?

Theme folders already get pretty cluttered with so many template files. Having the option of adding the screenshots to a sub-folder would be nice.

#19 in reply to: ↑ 18 @nacin
12 years ago

Replying to greenshady:

Will the extra theme screenshots (other than screenshot.png) be housed in the top level of the theme directory like we do with plugins? Or, will there be a /screenshots folder, or at least an option to filter it to do so?

Theme folders already get pretty cluttered with so many template files. Having the option of adding the screenshots to a sub-folder would be nice.

I would tend to agree, but that would mean that the initial screenshot would still need to remain the root, so then only extra screenshots end up in /screenshots.

#20 in reply to: ↑ 18 @Otto42
12 years ago

Replying to greenshady:

Will the extra theme screenshots (other than screenshot.png) be housed in the top level of the theme directory like we do with plugins? Or, will there be a /screenshots folder, or at least an option to filter it to do so?

Filter wouldn't work. If these shots are on the theme selection screen, then core needs to know their location without any code from the theme running.

For the moment, everything is geared towards having core-known-files be in the theme root. If we want to add subfolder functionality, then I think a better approach would be higher level than just the screenshots. Make the theme-file-searcher-thingamabob such that it's capable of looking in named subfolders as well as for named files and such.

For now, I'd stick with a root level naming scheme. screenshot-1.png, screenshot-2.png and so forth. This is simple to understand, works fine, and using the dash (-) makes it flexible to possibly allow for smarter theme-file-searching later (in the same way get_template_part uses the dash to separate things).

@helenyhou
12 years ago

#21 @helenyhou
12 years ago

19816.5.diff is a refresh against r20043

@helenyhou
12 years ago

#22 follow-up: @nacin
12 years ago

/screenshot\.(jpg|png)$/ should also include gif and jpeg, as those are supported extensions in core.

For theme installs, the API is set up to serve a screenshot_count field (int) and also a screenshots field (array of URLs).

I searched for "screenshot-2.png" site:themes.svn.wordpress.org, but couldn't find a single theme approved in the last two years that has more than one screenshot fitting the requirements. I am going to ping the theme reviewers list to see if we can get some themes operational with the new specs.

#23 in reply to: ↑ 22 ; follow-up: @cais
12 years ago

Replying to nacin:

/screenshot\.(jpg|png)$/ should also include gif and jpeg, as those are supported extensions in core.

For theme installs, the API is set up to serve a screenshot_count field (int) and also a screenshots field (array of URLs).

I searched for "screenshot-2.png" site:themes.svn.wordpress.org, but couldn't find a single theme approved in the last two years that has more than one screenshot fitting the requirements. I am going to ping the theme reviewers list to see if we can get some themes operational with the new specs.

I do not believe it has ever been considered prior to this ticket in the sense that why include files that for all intent and purpose would never be used.

We could put a call out to have themes submitted / updated with extra screenshots.

NB: I am preparing a "quick" fix (as noted in #19910) for Desk Mess Mirrored; I can include an extra screenshot of two for testing purposes if wanted.

#24 in reply to: ↑ 23 ; follow-ups: @nacin
12 years ago

Replying to cais:

I do not believe it has ever been considered prior to this ticket in the sense that why include files that for all intent and purpose would never be used.

A few themes did include extra screenshots anyway, but the only ones I found A) were older than two years, B) used jpg instead of png, or B) used Screenshot-2.png instead of screenshot.png.

We could put a call out to have themes submitted / updated with extra screenshots.

Sure. We will need to make a major push on that front. (We should probably allow updated screenshots to be quickly approved for previously updated themes.) But not until we have code in place to actually count the screenshots; that is not set up yet.

NB: I am preparing a "quick" fix (as noted in #19910) for Desk Mess Mirrored; I can include an extra screenshot of two for testing purposes if wanted.

That would be fantastic. 300x225 like screenshot.png. must be screenshot-2.png, -3.png, etc. Let me know once it is live so I can set up the data point.

#25 in reply to: ↑ 24 @cais
12 years ago

Replying to nacin:

Replying to cais:

We could put a call out to have themes submitted / updated with extra screenshots.

Sure. We will need to make a major push on that front. (We should probably allow updated screenshots to be quickly approved for previously updated themes.) But not until we have code in place to actually count the screenshots; that is not set up yet.

We can make that part of the Review-In focus on March 3, 2012 if there are any available for review.

NB: I am preparing a "quick" fix (as noted in #19910) for Desk Mess Mirrored; I can include an extra screenshot of two for testing purposes if wanted.

That would be fantastic. 300x225 like screenshot.png. must be screenshot-2.png, -3.png, etc. Let me know once it is live so I can set up the data point.

I should be able to have something later today ... just have to grab a few screens and crop them into the right size.

NB: I'll also note the screenshots in the theme's 'readme.txt' file in a similar fashion as a plugin's 'readme.txt' file re: screenshot description etc.

PS: @nacin - no screenshot-1.png?

Last edited 12 years ago by cais (previous) (diff)

#26 in reply to: ↑ 24 @cais
12 years ago

Replying to nacin:

Replying to cais: ... Let me know once it is live so I can set up the data point.

The latest version is in the theme trac: http://themes.trac.wordpress.org/ticket/6735.
I'll take the liberty to push it live momentarily.

I added three (3) additional screenshots named: screenshot-2.png, screenshot-3.png, and screenshot-4.png

#27 @emiluzelac
12 years ago

  • Cc emil@… added

#28 @helenyhou
12 years ago

There is investigation of some ways of making this part of the theme admin enhancements better going on, and we may very well end up with something a bit different from the current track. The UI now isn't beautiful, and the UX kind of sucks. The Jetpack admin screen, with its controlled content, is a sort of best case scenario of this kind of UI and it isn't necessarily perfect, either. Theme/author names and descriptions are not really restricted in terms of length.

Thinking we should reconsider including an inline image slider as a part of the UI - the hover-carousel thing on the larger screenshots in the Chrome Web Store seems like a good inspiration (minus the description slide-in - same issue as above with content length).

#29 @koopersmith
12 years ago

In [20426]:

Larger screenshots in theme list tables. see #20403, #19816.

#30 @helenyhou
12 years ago

  • Milestone changed from 3.4 to Future Release

#31 @helen
11 years ago

  • Reporter changed from helenyhou to helen
  • Type changed from task (blessed) to enhancement

#33 @ocean90
10 years ago

  • Milestone Future Release deleted
  • Resolution set to duplicate
  • Status changed from new to closed

Duplicate of #26695.

Outdated.

Note: See TracTickets for help on using tickets.