Opened 13 years ago
Closed 11 years ago
#19816 closed enhancement (duplicate)
Theme details should be able to include multiple screenshots
Reported by: |
|
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)
Change History (44)
@
13 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:
↓ 8
@
13 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.
#8
in reply to:
↑ 5
@
13 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.
#10
@
13 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.
#11
@
13 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.
#12
@
13 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.
@
13 years ago
Fixed Whitespace issue on expansion, clearing of details on resize, caching of $(this), code
#13
@
13 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.
@
13 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]
#15
@
13 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()
.
#16
@
13 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.
#18
follow-ups:
↓ 19
↓ 20
@
13 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
@
13 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
@
13 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).
#22
follow-up:
↓ 23
@
13 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:
↓ 24
@
13 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:
↓ 25
↓ 26
@
13 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
@
13 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?
#26
in reply to:
↑ 24
@
13 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
#28
@
13 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).
Using radio buttons to toggle through the screenshots.