bisq-network / bisq-website

@bisq-network website at https://bisq.network
34 stars 76 forks source link

Separate javascript into page-specific files and remove unused code #381

Closed m52go closed 4 years ago

m52go commented 4 years ago

I wanted to add an anchor link to the front page, but FAQ page JavaScript was getting in the way.

As it turned out, JavaScript code for the home and FAQ pages was being loaded for every page. This also made load times unnecessarily slower for other pages.

This PR puts JavaScript for the home page in a separate home.js file that is only loaded when the home page is loaded. Likewise for the FAQ page.

Additionally, it removes lingering code from when Google Analytics was used.

Review/test: https://deploy-preview-381--bisq-website.netlify.app/

Review Guidance

In this case, there are just 2 commits, so it's worth looking at them separately.

You should see js/scripts.js missing on every page, and js/home.js and js/faq.js added to all home pages and all faq pages...but everything else should be the same.

Bayernatoor commented 4 years ago

288eef8:

  1. Reviewed the code base - found no other mention of the sendAnalytic() function.

0605baa:

  1. Dynamic elements checked and working on local page and netlify, expanders and hover effect. (did notice that i couldn't switch to light mode, only dark mode was available).

  2. Compared _site dir from master against proposed branch (correct term? ). script.js has been removed and replaced and added to the js directory home.js and faq.js

Looks good to me ... let me know if i went about it wrong at any point :)

Not sure whether the light mode/dark mode not working is an issue.

m52go commented 4 years ago

(did notice that i couldn't switch to light mode, only dark mode was available).

This was crucial. Thank you for finding this. Yes I completely removed the code for the toggling dark mode when deleting scripts.js...the code for the toggle was not commented and kind of hidden at the bottom, so I didn't see it.

I've added it back now and rebased the branch so it's included in the last commit (https://github.com/bisq-network/bisq-website/pull/381/commits/ec7f8eaad7f9d3413e6499211fd3c07bc78c8368).