Fix #152 Show ST4 badges

wbond / packagecontrol.io

The Package Control website

https://packagecontrol.io

Other

111 stars 46 forks source link

Fix #152 Show ST4 badges #161

Open gerardroche opened 1 year ago

gerardroche commented 1 year ago

Show ST4 badges for packages that support ST4. Previously no badge was shown for packages that support ST4. This fixes #152
Make ST4 badges green like the ALL badge. Previously only the ALL badge was green, but I think the latest version should also be green.
Show ALL badge on package list. Previously no badge was shown on the listing page for packages that support all versions.

Fix #152

Q. Do I need to include the app.css and app.js files?

Listing before

listing-before

Listing after

listing

Package page before

package-before

Package page after

package-after

FichteFoll commented 1 year ago

I think this needs a bit of a discussion. Technically, this is a correct approach, but I'd argue that supporting the latest version is the expected behavior and does not need to be highlighted explicitly and ST2 support will be removed on the next PC release (finally), so I propose the following:

package supports ST 3 and ST 4: no badge
package supports ST 4 only: has ST4 badge or has "no ST3" badge
package supports ST 3 only: has ST3 badge

This is basically the same as when we had ST2 and ST3 in parallel. Since ST2 is still technically supported by the latest version (though not really?) and we still have packages listed for it, I would also be ok with showing both ST3 and ST4 badges for the ~~second~~first case here until that is finalized. However, I don't think we need "ALL".

gerardroche commented 1 year ago

If we do away with the all then the st4 badge doesn't need to be green. The only reason I made that green is because the ALL tag seem to shadow the ST4 one which I don't think is right.

[x] package supports ST 3 and ST 4: no badge Yes. Because this kind-of implies ALL. ST2 is so old that ST3 and ST4 is pretty mush ALL.
[x] package supports ST 4 only: has ST4 badge or has "no ST3" badge I think just the ST4 tag for this one.
[x] package supports ST 3 only: has ST3 badge Yes.

I think that makes sense and is much cleaner. Will I go ahead and make that change?

FichteFoll commented 1 year ago

Will I go ahead and make that change?

SGTM. I can't merge or deploy any of this, though, since only wbond has the permissions to do so.