Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Front-end clean-up #420

Open
wants to merge 1 commit into
base: alpha
Choose a base branch
from

Conversation

HebaruSan
Copy link
Contributor

@HebaruSan HebaruSan commented Oct 31, 2021

Background

For a while I had the growing impression that SpaceDock's front-end dependency handling was messier than it needed to be. Every now and then I would see what looked like an edit to a Bootstrap file in our git log and assumed the worst without really wanting to look into it further.

See the comments in #391 for an example of such confusion; I thought the weird menu background on mobile was caused by Bootstrap itself because it was in a file called bootstrap-theme.css (which is the name of a file shipped by Bootstrap), but then I noticed that @V1TA5 had added that line in April 2016. I assumed that we had been editing copies of released files from other projects, which would have been a huge red flag for maintenance.

Eventually I created #405 as a reminder to look into this in more depth.

Investigation findings

However, when I dug into the git log, a somewhat different picture emerged:

  1. The only suspicious file that had been edited was bootstrap-theme.css; all the other static dependencies embedded in the repo were identical to their originally committed versions 😌
  2. bootstrap-theme.css is meant to be a user-owned config file! There are packages in npm containing nothing but alternate versions of this file for different themes, and in fact our own copy says "Generated using the Bootstrap Customizer", so it probably came from an interactive web tool's download button after various customizations were made. So it is fine to commit this file to our repo and edit it.

Still, there is room for improvement here. Committing other projects' files to our repo is a bit sloppy, it makes updating dependencies (or sometimes just identifying them) more difficult, and some of the styling in bootstrap-theme.css doesn't really relate to theming Bootstrap.

Changes

  • Files not created or owned by SpaceDock are removed from frontend/static/css and frontend/static/js (and a few are removed from frontend/static/fonts). In most cases, the ones we actually use are now installed by npm and then copied from node_modules to build/ by build.js. This means we can manage front-end dependency versions with npm, so upgrades are easier.
    • The exceptions are the stylesheets from Bootstrap and FontAwesome, which are installed by npm but then imported into a new file frontend/styles/bundle.scss so we can set their font paths to /static
    • The editor.js we were using does not seem to have a compiled version available in npm and is essentially unmaintained, so it is replaced by https://www.npmjs.com/package/@devhau/md-editor, which is a pretty nice drop-in replacement with a slight WYSIWYG vibe
    • Other dependencies' versions are updated, which broke dropzone (contrary to npm's promise that the ^ prefix means "Compatible with version") until we grabbed the .Dropzone object out of it instead
    • frontend/static/css/create.css was a compiled version of frontend/styles/create.scss and is deleted
    • frontend/static/css/bootstrap_dark.min.css was not in use and is deleted
  • frontend/static/css/bootstrap-theme.css now contains only generic theming that could apply to any Bootstrap-based site
    • Styling for widely-used SpaceDock-specific elements is moved to frontend/styles/stylesheet.scss
    • Styling for the mod box is moved to frontend/styles/listing.scss
    • A few invalid lines are removed, for example where a number and its unit had a space between them (as in 5 mm), which is not valid CSS. They can be re-added and tested properly if we decide we want them.
    • The unused flipper class is moved to frontend/styles/flipper.scss in case we decide to revive it. It makes the mod box rotate around on hover, which could be either neat or annoying. It's not clear whether this was planned or debated at some point in the past or just not finished. I posted an animation of it in the chat and am waiting for feedback.
  • /static/stylesheet.css is now included for all pages by layout.html instead of being pulled in by many different (but not all) templates
  • build.js now minifies our generated scripts and removes whitespace from our stylesheets
  • Templates, stylesheets, and scripts are renamed to <noun>_<verb>.<extension> for consistency and more convenient sorting (with view treated as the default, absent verb)
    • Modpacks are now called "packs" everywhere instead of "lists" sometimes
  • The box sizing and flipping code is moved out of layout.html and into global.coffee
  • The mod page's div.header previously had a background: transparent; style which was immediately overridden by background-color: #FFFFFF;. In practice all this accomplished was to mess up the layout of the header graphic. This is now removed so the header graphic will appear as it was intended to.

Now if we want to upgrade Bootstrap (or any other dependency), switch to a fresh Bootstrap theme, or add new front-end dependencies, all we will have to do is run a few npm commands and start coding. An up to date site is a happy site.

Fixes #405.

@HebaruSan HebaruSan added Type: Improvement Area: Backend Related to the Python code that runs inside gunicorn Priority: Normal Status: Ready Area: Frontend Related to HTML, JS, CSS, or other browser things labels Oct 31, 2021
@HebaruSan HebaruSan linked an issue Oct 31, 2021 that may be closed by this pull request
@HebaruSan HebaruSan added the Scope: Large Complex changes requiring a lot of effort to develop and review label Nov 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: Backend Related to the Python code that runs inside gunicorn Area: Frontend Related to HTML, JS, CSS, or other browser things Priority: Normal Scope: Large Complex changes requiring a lot of effort to develop and review Status: Ready Type: Improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Tech debt] Purge the Bootstrap hacks (finally)
1 participant