0
\$\begingroup\$

This is my working template file which I manually edit to create new properly formatted webpages for my personal website.

I am interested in what advantages if any could be realized by placing the CSS in this file into an external file before modifying this file to work as a Django template.

Should I just leave it the way it is before I add Django template tags to this file?

Here is my present template code below:

<!DOCTYPE html>
<html>
<head>
<style>
div.in_page_menu {
    background: radial-gradient (#f69d3c,#ffeeaa);
    opacity: 0.6;
    margin: 50px;
    font-size: 14px;
    border-width: 25px;
    border-top-style: none;
    border-right-style: none;
    border-bottom-style: none;
    border-left-style: solid;
    border-color: hsl(0, 0%, 73%);
    box-shadow: 0 9px 9px -5px #666;
}
</style>
<style>
div.contentbox {
    background: #ffeeaa;
    opacity: 0.7;
    margin: 50px;
    font-size: 28px;
    border-width: 25px;
    border-top-style: none;
    border-right-style: none;
    border-bottom-style: none;
    border-left-style: solid;
    border-color: hsl(0, 0%, 73%);
}
</style>
<style>
div.body_div {
  background-image: url("butterflies_faded.png");
  background-repeat: no-repeat;
  background-attachment: fixed;
}
</style>
<script src="autoScrollTo.js"></script>
<link rel="icon" href="favicon.ico" type="image/x-icon">
<link rel="stylesheet" href="style/style.css">
</head>
<body>
<?php include_once("php_includes/template_pageTop.php"); ?>
<div class="body_div">
<h2 id="myheading">Resource Links</h2>
<div class="in_page_menu">
<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 />
<a href="#" onclick="return false;" onmousedown="autoScrollTo('div3');">
  text link 3</a><br />
<a href="#" onclick="return false;" onmousedown="autoScrollTo('div4');">
  text link 4</a><br />
  <a href="#" onclick="return false;" onmousedown="autoScrollTo('div5');">
  text link 5</a><br />
<a href="#" onclick="return false;" onmousedown="autoScrollTo('div6');">
  text link 6</a><br />
<a href="#" onclick="return false;" onmousedown="autoScrollTo('div7');">
  text link 7</a><br />
  <a href="#" onclick="return false;" onmousedown="autoScrollTo('div8');">
  text link 8</a><br />
  <a href="#" onclick="return false;" onmousedown="autoScrollTo('div9');">
  text link 9</a><br />
<a href="#" onclick="return false;" onmousedown="autoScrollTo('div10');">
  text link 10</a><br />
 </div>
 
 
<div id="div1" class="contentbox">
<h3>content 1 heading</h3>
<p>
paragraph content 1
</p>
<ul>
    <li>content 1 list item
</ul>
<a href="#" onclick="return false;" onmousedown="resetScroller('myheading');">
  go back to top</a>
  <br>
  <br>
<div id="div2" class="contentbox">
<h3>content 2 heading</h3>
<p>
paragraph content 2
</p>
<ul>
    <li>content2 list item
</ul>
</div>
<a href="#" onclick="return false;" onmousedown="resetScroller('myheading');">
  go back to top</a>
<div id="div3" class="contentbox">
<h3>content 3 heading</h3>
<p>
paragraph content 3
</p>
<ul>
    <li>content 3 list item
</ul>
</div>
<a href="#" onclick="return false;" onmousedown="resetScroller('myheading');">
  go back to top</a>
<div id="div4" class="contentbox">
<h3>content 4 heading</h3>
<p>
paragraph content 4
</p>
<ul>
    <li>content 4 list item
</ul>
    </div>

<a href="#" onclick="return false;" onmousedown="resetScroller('myheading');">
  go back to top</a>
  <div id="div5" class="contentbox">
  <h3>content 5 heading</h3>
  <p>
paragraph content 5
</p>
<ul>
    <li>content 5 list item
</ul>
</div>
<a href="#" onclick="return false;" onmousedown="resetScroller('myheading');">
  go back to top</a>
<div id="div6" class="contentbox">
<h3>content 6 heading</h3>
<p>
paragraph content 6
</p>
<ul>
    <li>content 6 list item
</ul>
</div>
<a href="#" onclick="return false;" onmousedown="resetScroller('myheading');">
  go back to top</a>
<div id="div7" class="contentbox">
<h3>content 7 heading</h3>
<p>
paragraph content 7
</p>
<ul>
    <li>content 7 list item
</ul>
</div>
<a href="#" onclick="return false;" onmousedown="resetScroller('myheading');">
  go back to top</a>
<div id="div8" class="contentbox">
<h3>content 8 heading</h3>
<p>
paragraph content 8
</p>
<ul>
    <li>content 8 list item
</ul>
</div>
<a href="#" onclick="return false;" onmousedown="resetScroller('myheading');">
  go back to top</a>
  <div id="div9" class="contentbox">
  <h3>content 9 heading</h3>
  <p>
paragraph content 9
</p>
<ul>
    <li>content 9 list item
</ul>
</div>
<a href="#" onclick="return false;" onmousedown="resetScroller('myheading');">
  go back to top</a>
<div id="div10" class="contentbox">
 <h3>content 10 heading</h3>
 <p>
paragraph content 10
</p>
<ul>
    <li>content 10 list item
</ul>
</div>
<a href="#" onclick="return false;" onmousedown="resetScroller('myheading');">
  go back to top</a>
</div>
</body>
</html>
```
\$\endgroup\$

2 Answers 2

Reset to default
3
\$\begingroup\$

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');
  });
}
\$\endgroup\$
2
  • \$\begingroup\$ I don't know how to implement the improved "DRY" javascript you have given. I don't see how to feed in the correct number of links. Also where is the html link for the user to select autoScrollTo? \$\endgroup\$
    – stacker
    Commented Nov 27, 2020 at 22:07
  • \$\begingroup\$ To implement it, just remove your inline handlers and run the JavaScript in the answer instead, to add the event listeners. Since the nth link is meant to link to the nth .contentbox element, use the index of the a being iterated over to identify the corresponding .contentbox to go to - the second parameter of forEach is the index being iterated over. \$\endgroup\$
    – CertainPerformance
    Commented Nov 27, 2020 at 22:10
0
\$\begingroup\$

Separating styles and templates really matters when you have large templates and many of them. As soon as your site starts to grow, it becomes increasingly unmaintainable to have both mixed up.

One minor reason is that your templates become larger than required. If you want to make a quick change in a template you have to scroll past all the noise at the top of the file first.

But the main drawback is that you cannot reuse styles across templates. You have to define certain parts of your styles over and over again. This duplicates make the style much harder to change because you have to make sure you consistently make the changes at all the places.

I see hardly any reason why you would combine css and html in one file. It doesn't hurt at all to separate the right from the beginning.

By the way, you don't have to start a new <style> tag for each CSS selector. One opening <style> at the top and a closing </style> at the bottom of your CSS is enough.

Update: Having read @CertainPerformance's answer I think it's important to point out that there may be (performance) reasons to place your CSS inside the HTML for the browser but it doesn't mean you shouldn't organize your source files properly and separate templates and styles. If required, you can use tools such as webpack to bundle everything together and send just one file to the client.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.