Support a little more data in .info files

[jenlampton]

This is a spin-off from https://github.com/backdrop-ops/backdropcms.org/issues/100

I have added several fields to the project content types on backdropcms.org:

1. unlimited value term reference field "Project Tags" -- the list of tags has been seeded with the tags listed in the documentation
2. unlimited value image field "Screenshots" -- this will replace our "Image" field since it is multiple value. The image we pass to project browser will be the first (delta = 0) of this list.
3. unlimited value short text field "GitHub Maintainers" -- since right now all we have are maintainers on GitHub, this was the safest way to handle these values. If in the future we support other kinds of maintainers, an update hook will be easy if we treat all values the same way right now.
4. unlimited value List (text) field "Theme Colors" -- this currently supports any of the following colors:

Aqua
Beige
Black
Blue
Brown
Gold
Gray
Green
Ivory
Lavender
Lime
Maroon
Navy
Olive
Orange
Pink
Purple
Red
Salmon
Silver
Tan
Teal
Violet
White
Yellow

I tried to pull all the un-ambiguous color names from the list of CSS colors, but let me know if we need to pair this down further. (do we really need either/both lavender and violet? what about Tan and Beige? Acqua and Teal?)

---

~~PR: https://github.com/backdrop-ops/backdropcms.org/pull/488~~ https://github.com/backdrop-ops/backdropcms.org/pull/750

[docwilmot]

@jenlampton @quicksketch PR up, but caveat: I cant really test it. Like I mentioned, I could create fake GitHub API event to test locally but haven't actually been able to figure out how to do a real webhook to WAMP, and had to comment out all mention of backdrop_http_request() during testing locally. So would be grateful if you could handle the finishing touches.

The actual updating of fields works though.

Also note the https://github.com/backdrop-ops/backdropcms.org wasnt updated after you added the new fields to the content types. So I dont know how that will affect merging, but look out for that.

[quicksketch]

I left some feedback at https://github.com/backdrop-ops/backdropcms.org/pull/488#pullrequestreview-187168653.

One thing that's not clear to me: what happens if screenshots[] is specified in a theme's .info file AND there is a screenshot.png file. Are both used? Or does screenshots[] take precedence? I would think that if specified, screenshots[] would be used on backdropcms.org. The screenshot.png in the theme directory would then only be used on the Appearance page.

[docwilmot]

Feedback is all very reasonable, will do shortly.

Are both used?

I left in the code for screenshot.png since it may take a while for themes to be updated (not that we have that many) but I imagine the new screenshots[] deprecates that. I suspect @jenlampton will need to do a .tpl.php file for the screenshots field with a if (!screenshots): screenshots = screenshot.png

We'll need to update Backdrop core to do the same.

[jenlampton]

I suspect @jenlampton will need to...

👍 can do!

[jenlampton]

A thought: we'll need the packager to remove the screenshots directory after sending the files over to backdropcms.org.

[docwilmot]

I didnt remember about this detail.

That would require a change in the thinking behind the code. Currently screenshots[] = CAN_BE_ANY_DIRECTORY/image.png_or_jpeg. We'll need to only delete the images.

But we'd need to think through how the admin/appearance page uses this. Do we allow BackdropCMS to pick a random one of the images and package that with the upload? Or recommend at least one image have a special name that would tell the packager to ship it with the upload ( screenshot.png for example)? Or do we forego an upload completely and just embed an image directly from BackdropCMS (like Installer does, but then if youre offline there'd be no preview)?

For now, since we're keeping screenshot.png most themes would have no problem, but we'd need to decide before we deprecate that.

[docwilmot]

We also cant do this with any existing hooks. We could add a backdrop_alter() to https://github.com/backdrop-ops/backdropcms.org/blob/7e85f08ec395caa6098d2f6149f52d447bdc620d/www/modules/contrib/project/project_github/project_github.pages.inc#L55 to modify the $files before theyre uploaded. This would require a new release of Project module first I imagine. @jenlampton or @quicksketch will need to decide on this.

[klonos]

I would prefer if there was a single screenshot file left as fallback when offline.

[docwilmot]

Can we get feedback on this? @quicksketch can you comment on any way to make Project clean up the screenshots apart from adding new hook(s) to Project?

I would prefer if there was a single screenshot file left as fallback when offline.

My idea was:

• allow screenshots in any directory with the screenshots[] = where to locate them
• for now for the appearance page, use screenshot.png from the root of the theme package as usual
• eventually GitHub would choose an image for the appearance page
  • use an image called screenshot.png if it exists in the theme root
  • use an image called screenshot.png if it exists anywhere else
  • if not use the first image in the screenshot[] = list
• GitHub packager could then move that image to the root as usual so all themes downloaded from GitHub would have that as usual. There would be a discrepancy though in that modules on Github would have all images in the theme, but downloaded images would only have one screenshots.png.
• the appearance page would always then continue to use screenshots.png
• Project Installer would be able to display all the screenshots still by pulling them from BackdopCMS.org directly

[jenlampton]

Do we allow BackdropCMS to pick a random one of the images and package that with the upload?

I think it would be smarter to always pick delta = 0. People will learn that the first image they provide is the one that will be used.

allow screenshots in any directory

Since we are encouraging everyone to use the same directory names in all projects for css js config and libraries I don't see why we shouldn't just assume (and enforce) use of a screenshots directory. It will make for better DX if it's consistent across all projects.

The list of screenshots[] would then contain the file names (within the screenshots directory), and the order would determine the delta. The first image in this list would always be the one used as the screenshot on the project page, and would be the one image that's not removed by the packager. Or it could even be renamed screenshot.png and placed in the project root if that file did not already exist.

[docwilmot]

this is reasonable. now just to figure out https://github.com/backdrop-ops/backdropcms.org/issues/487#issuecomment-457367120, how to get Project to delete images. As mentioned I cannot see how it could be done neatly without changes to Project module.

[docwilmot]

@jenlampton I finished this, then had a thought: if we're going to mandate a screenshots directory, whats the point of the screenshots[] = xxx array?

The way theme info files work now is that if you dont want your screenshot to be called screenshot.png or if you dont want it in your theme root, you can put screenshot = path/to/it in your info file. So people are going to be used to be able to have their screenshots where they wish, and declare this in their info files. Like is you want to keep your screenshot in your images folder, you can do that now in D7 and Backdrop.

So this is why I had though that screenshots[] = path/to/them would work the same.

In either case if we want to mandate a screenshots directory, this becomes superfluous.

[jenlampton]

Interesting. I was thinking that the first one listed would be the primary one (the one that shows up in the project installer, etc) but maybe we could allow a single line that defines only the featured one?

[docwilmot]

So enforce the screenshots folder, AND require a line in the info file to say which one it's the main screenshot?

That would be changing expected functionality a lot though.

[jenlampton]

That would be changing expected functionality a lot though.

Would it? We already have a line in the info file to say which one is the main screenshot.

All we'd be doing is adding/enforcing a screenshots folder for all the other images.

[docwilmot]

by this I mean:

• we will lose the ability to specify a preferred location for screenshots and force the screenshots folder as the only option
• we will now require an entry in the info file whereas it was previously optional, since there is no way to "order" images in a folder, except alphabetical

personally I dont think this is a major issue since I doubt 80% of theme developers are actually utilizing the first point above, and the second point is going to be necessary regardless, if we're allowing multiple images. but thought it needed saying that we'd be losing existing (if not expected) functionality.

[docwilmot]

I just thought as well though. Its probably likely that the image a theme uses for the screenshot may not be the one we want to show on b...org, since its going to be low resolution? so the screenshots array in the info file is still necessary, so theme devs can decide what order their images on b..org show up.

and do we mandate a maximum number of images? we dont want theme listings on b..org having 16 images.

more questions exist, i'm sure.

@klonos?

[jenlampton]

we will now require an entry in the info file whereas it was previously optional, since there is no way to "order" images in a folder, except alphabetical

We can still leave it as optional, alpha is a suitable fallback. If someone notices the wrong default image is showing they will likely look into how to change it.

Another thought: what if they want their screenshots to all be in a specific order? Do we want to make them rename their files so that they alphabetically match their preferred order? Or would it be better to have them define all of them as part of a screenshots[] array?

[docwilmot]

Another thought: what if they want their screenshots to all be in a specific order? Do we want to make them rename their files so that they alphabetically match their preferred order? Or would it be better to have them define all of them as part of a screenshots[] array?

you might have missed my last comment:

so the screenshots array in the info file is still necessary, so theme devs can decide what order their images on b..org show up.

[jenlampton]

You might have missed my reply to your last comment:

We can still leave it as optional, alpha is a suitable fallback. If someone notices the wrong default image is showing they will likely look into how to change it.

My recent question was about all the other screenshots. :)

[docwilmot]

Summary for anyone reading this (and to make it clearer in my mind):

We are planning to have more info in .info files, and thats come along nicely, we're almost there. One new sugegstion was to have a screenshots[] = xxx array to allow multiple screenshots for display on backdropCMS.org.

But the plan will be that once a release of a theme is made, we will delete the screenshots folder from GitHub so that users downloading the theme from GH wont waste bandwidth on images their site will likely never use as there will still only be one screenshot on the site themes page. Questions arise though:

• currently you can add a line in your info file to specify what the name and location of you screenshot is. Do we want to do the same for the screenshots folder? Or for individual screenshots?
• or do we mandate a screenshots folder in your theme root, with the screenshot list in the info file and deprecate the ability to have your screenshot stored wherever you want?
• how do we tell sites downloading this theme, which image is the standard site admin/appearance screenshot in our array and which ones we want to display on BackdropCMS.org? We were thinking the first one in the array would be the screenshot, but the screenshot would need to be small resolution (its usually only 294 x 219px but doesnt have to be) so would we skip it on BackdropCMS.org?
• (just thought of this) If I create a theme foo, one master branch only, and create a release, how do we delete the screenshots folder from GH wihtout a commit to note that we did that? And if the folder is gone, what happens next release? how would BackdropCMS.org handle the (now missing) screenshots folder. I suspect I dont really know how this could all work.
• would we want a maximum number of images?

Anything else

[ghost]

Thanks for the summary @docwilmot!

I see two main issues here:

1. Where to store screenshots
2. How to determine which is used where

For the first issue, I really prefer consistency over (most) other things. The Creating modules API page says:

JavaScript files are always kept in a /js folder, CSS in a /css folder, templates in a /templates folder, and tests in a /tests folder, as demonstrated in My Module above.

Granted, this is referring to modules, but it should (if not does) apply to themes too. Therefore, if we mandate a specific directory to use for CSS, JS, etc. files, we should do the same for screenshots (consistency FTW).

As for the second issue, screenshots that appear on BackdropCMS.org are just that - screenshots showing the theme in action; how it looks at different screen sizes, etc. But often what will be shown for a theme on the admin/appearance page in Backdrop itself will be a logo, or part of the design; generally not a full-page screenshot.

I therefore propose having two settings to account for this difference:

• screenshots[] = xxx: an array of large images in the screenshots folder that are used on BackdropCMS.org for advertising the theme, and which are removed/not added to the zip packages on GitHub.
• image = yyy: a single, small logo/cropped screenshot/image in the theme's root folder which is displayed on the admin/appearance page in Backdrop when enabling the theme.

[docwilmot]

Thank you @BWPanda thats been quite helpful. I agree with your evaluation re the screenshots folder (which is what I think we were leaning towards).

Re the image for the appearance page, I think a simple solution could be adding array keys, like screenshots[default] = xxx or screenshots[logo] = xxx. I'll code that and we'll work on the key name later.

Reminder we'll have to remove the code that allows moving the screnshot in a core PR, once this is done.

And never mind re my fourth question about, that isnt a problem, its the release zip, not the repo that we're fiddling with, so all is well there.

[docwilmot]

@jenlampton please see latest PR on this issue. I think I've gone as far as I can without being able to test this. And reminder I cant test this because I dont have the GitHub webhooks pointing at my local.

[jenlampton]

@quicksketch can you let us know how we can test this? When you tested this last time, what process did you use (and should we do the same?)

[klonos]

Sorry for coming late to the party...

and do we mandate a maximum number of images? we dont want theme listings on b..org having 16 images.

Why not?

Another thought: what if they want their screenshots to all be in a specific order? Do we want to make them rename their files so that they alphabetically match their preferred order? Or would it be better to have them define all of them as part of a screenshots[] array?

you might have missed my last comment:

so the screenshots array in the info file is still necessary, so theme devs can decide what order their images on b..org show up.

So I think that:

• If people simply add images to a screenshots directory, then we pull these and render them in alpha (as the parse order). This provides an easy, kinda automated way, for those not fussed about the order of the screenshots.
• If the maintainers care about the order of their screenshots, then they can manually set it by the optional screenshots[] = lines in their .info files.

...the plan will be that once a release of a theme is made, we will delete the screenshots folder from GitHub so that users downloading the theme from GH wont waste bandwidth on images their site will likely never use as there will still only be one screenshot on the site themes page. Questions arise though

Do you mean that the plan is to have the packaging script remove those from the .zip? If so, then yes; if you literally mean delete the files from GitLab, then please no.

...if we mandate a specific directory to use for CSS, JS, etc. files, we should do the same for screenshots (consistency FTW).

Yes please 👍

[klonos]

...never mind re my fourth question about, that isnt a problem, its the release zip, not the repo that we're fiddling with, so all is well there.

😌 ...yes. This is the way to go: leave the actual files on the repo intact; clean things up in the package.

Re which image should be the default/logo, I think that:

• Any image with a specific filename (say thumbnail.whateverextension ??) should serve as what is shown in admin/appearance, and in the theme listing in the Project Browser, and on b.org.
• If no thumbnail.xyz file is found, then whatever is delta = 0 from those other images in /screenshots

[stpaultim]

I really like the intent of this PR. I am particularly interested in the idea of including maintainers (#3 in original proposal), if that is still included.

It's not clear how I can help with this issue. Maybe we need another infrastructure sprint week sometime soon when we can focus on issues like this.

It sounds like the hold up might be finding a way to test this PR. Was there some talk about using Tugboat to test PR's for backdropCMS.org?

[ghost]

Was there some talk about using Tugboat to test PR's for backdropCMS.org?

Yes. The Forum repo now has Tugboat PR sandboxes. The API and B.org repos are yet to come.

[docwilmot]

So I got ngrok working locally after some coaxing and tested the PR again. Everything until the screenshots works fine (so I assume this will work fine too). I should be able to push a well reliable PR shortly. But then I am confused again as to how to handle screenshots. The PR expects both a screenshots folder and entries in the .info file listing the filenames. But if we mandate a specific location for screenshots, then the info file entries (screenshots[]='' etc) are redundant I'm thinking. Those info file entries were only considered necessary because I initially considered allowing users to put their screenshots anywhere and then tell where they are through screenshots[]=path.

So how about the following options:

1. project has no screenshots folder

• the screenshot.png file will be in the root, and if not available whatever fallback exists now

2. project has a screenshots folder

• screenshot.png is optional. user can specify an entry in the info file saying screenshot = file.png to use one from the folder as default.
• if no default specified, use the "first" one in the screenshots folder.
• if a screenshot.png is also present in the folder or in the root, use that as default.

TO explain the concept of a default in case its not clear, there needs to be a file that's used on the /appearances page; it needs to be a specified size; currently Backdrop will use the file named screenshot.png. But if we're now going to allow multiple screenshots, and most will be large images to display on the BackdropCMS.org theme listing, we need a way to tell the packager which one is the little one to use from the screenshots folder. Thats what I'm calling "default". During release, the packager will create this file from one in the folder either if specified in the info file or the "first" image if not specified. All other images, including the screenshots folder in entirety will be deleted from releases though present in the GH code.

ANy objections?