javascript – Template file for creating formatted webpages

I am interested in what advantages if any could be realized by placing the css in this file into an external file

A tradeoff is involved:

  • If you put CSS in a separate file, browsers will be able to cache it easily (if your server is set up correctly). In contrast, if you serve the CSS inline, it’ll be sent over the wire every time the page is loaded. But…
  • If you put the CSS in a separate file, the first time the client visits the site, they’ll have to make two requests for the page to display properly: one for the HTML, and one for the CSS. (This same exact reasoning applies to <script>s)

For professional sites, the usual recommendation is to put CSS that’s critical to initial display inline, and to put secondary CSS in a separate file. Here, since your CSS rules are applying to elements immediately visible on the page, inline rules are probably a good choice.


Review

Combine common CSS rules when possible – DRY code is more elegant and easier to understand, usually. Here, since the margin and all the border- styles are the same for the .in_page_menu and the .contentbox, consider giving them both the same class with those rules, rather than repeating the rules twice. You could also do this via CSS preprocessing, like with SASS.

Combine tags – unless there’s a good reason for separate <style> tags, since they all refer to the same document, it’d make more sense to have just a single tag, not two or three.

Don’t block loading with scripts – the <script src="autoScrollTo.js"></script> in the <head> is blocking; the page won’t render until that script is downloaded, which could be a problem on bad connections who haven’t visited the site before. If autoScrollTo runs anything that needs to run ASAP on pageload, consider putting that content inline into another script tag in the HTML. Allow the browser to continue processing the document while not-immediately-essential scripts are being downloaded by giving those scripts the defer attribute.

Numerically indexed IDs are WET and inelegant – IDs should be reserved for elements that are going to be absolutely unique in a document, such as the #myheading. Items that are part of a collection without something special distinguishing one from the rest probably shouldn’t have IDs.

Avoid inline handlers, they’re
terrible given their scoping rules and quote escaping. Nowadays, there isn’t really any reason to use them – attach event listeners properly using Javascript with addEventListener instead.

Change:

<a href="#" onclick="return false;" onmousedown="autoScrollTo('div1');">
  text link1</a><br />
<a href="#" onclick="return false;" onmousedown="autoScrollTo('div2');">
  text link 2</a><br />
....

to:

const contents = document.querySelectorAll('.contentbox');
document.querySelectorAll('.in_page_menu a').forEach((a, i) => {
  a.addEventListener('mousedown', () => {
    autoScrollTo(contents(i));
  });
});

(changing autoScrollTo as needed to take a reference to the element instead of an ID string)

You could also consider styling the <a>s as blocks to remove the need for the <br />s between them.

You can also change all of the:

<a href="#" onclick="return false;" onmousedown="resetScroller('myheading');">
...

to

for (const a of document.querySelectorAll('.contentbox a')) {
  a.addEventListener('click', () => {
    // maybe change this to a direct reference to `#myheading instead of a string
    resetScroller('myheading');
  });
}