kbase / KbaseAppCatalogStatic

MIT License
0 stars 3 forks source link

Fix in the context of the new marketing and doc site #24

Closed eapearson closed 3 years ago

eapearson commented 3 years ago

A handful of important files, stylesheets, javascript files, and images, were not installed locally but referenced by url from the old kbase.us Wordpress site. This caused the visual aspect of this app to fail (it still fetched the catalog, but the layout was broken.)

I cherrypicked the files from a copy of the Wordpress site, installing them locally in the repo and updating the urls. Also added a favicon, which was missing, and replaced the horizontal menu with the title "App Catalog", and removed the search input. Replaced the problematic and oh-so-2015 bootstrap header with a custom, minimal flexbox implementation.

Needed to add a missing Python dependency (which tells me someone else must have deployed a patched copy of the repo, because it would never have worked), and updated other dependencies.

eapearson commented 3 years ago

Actually, I have one more commit coming, don't merge yet.

lgtm-com[bot] commented 3 years ago

This pull request introduces 8 alerts when merging e5b8337483bb2ba245afd6f4f2b1ce2585bc7f8d into 813f6a6e70698c1b611252a173968a8cf001a649 - view on LGTM.com

new alerts:

eapearson commented 3 years ago

BTW I would certainly make more changes to this repo, but this set of changes was really just to get it back on its feet when the Wordpress site is replaced. If this site is to last much longer, I would replace all loading from CDNs with local code, and have all JS dependencies installed via npm/yarn and update all of them to the most up-to-date versions or equivalents. There be monsters lurking in years-old javascript libraries. And, yes, copying JS code around like that is terrible, but no worse than it was approved for release before. It should certainly address issues mentioned by lgtm, etc.