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

Convert site from Bootstrap4 to Bootstrap5 #159

Merged
merged 6 commits into from
Jun 25, 2021
Merged

Convert site from Bootstrap4 to Bootstrap5 #159

merged 6 commits into from
Jun 25, 2021

Conversation

itsthejoker
Copy link
Member

Relevant issue: #145

Description:

Since we started working on this so long ago, it still uses bootstrap4 -- it's better to change over sooner rather than later so that we have fewer things to break. This PR:

  • removes bootstrap4
  • expands the generic form
  • adds error handling views for 404 and 500
  • fixes Plausible.js metrics loading
  • fixes navbar support for engineering blog
  • fixes navbar support for mobile

Turns out that adblock lists are literally just looking for a script called plausible.js, no matter where it's loaded from -- so on the recommendation of Plausible, we'll now self-host the script and I've changed the name to potentially.js, which, hilariously, is enough to make it load. I've also disabled it if you're logged into the site, since we don't really want to track page views on the edit pages.

Screenshots:

Homepage:
image

Example page:
image

New form:
image

Engineering main page:
image

Engineering blog post detail:
image

Also adds a new feature where the name dynamically collapses on the engineering blog side for desktop -> tablet -> mobile.

Desktop:
image

Tablet:
image

Phone:
image

Checklist:

  • Code Quality
  • Pep-8
  • Tests (if applicable)
  • Success Criteria Met
  • Inline Documentation
  • Wiki Documentation (if applicable)

@itsthejoker itsthejoker requested a review from a team as a code owner June 18, 2021 06:05
Copy link
Contributor

@TimJentzsch TimJentzsch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The site itself looks very nice.

However, I would recommend not to add JQuery now, because otherwise we'll forget about removing it.

Additionally, I have my doubts about circumventing adblockers for the tracking.

blossom/templates/website/generic_form.html Outdated Show resolved Hide resolved
@TimJentzsch
Copy link
Contributor

Just putting this comment here as a reminder to remove the testing form from the code

* Replace JQuery in input file and error message code

* Replace JQuery in form validation script

* Fix event listeners and clean up code

* Remove JQuery script

* Use query selectors for easier iteration

* Make error list clearing more efficient

* Use template string literals instead of concatenation

Co-authored-by: David Alexander <TheLonelyGhost@users.noreply.github.com>

* Fix form labels being deleted when displaying form errors

Co-authored-by: David Alexander <TheLonelyGhost@users.noreply.github.com>
@itsthejoker
Copy link
Member Author

Note to self: when this is deployed, we will have to manually fix the donations page.

@itsthejoker itsthejoker merged commit e956e40 into master Jun 25, 2021
@itsthejoker itsthejoker deleted the bootstrap5 branch June 25, 2021 05:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants