4
\$\begingroup\$

Recently I updated a JavaScript file for my website. I added in use of cookies. Bear in mind that I only just started to learn about cookies and it had errors. So I moved code around to stop it from having errors. However, I'm not sure if this is efficient.

Sometimes when I refresh my page it will show up with no styling for a split second. Even though it is only minute I would like to see if I can rid this problem. Is it efficient?

document.addEventListener ("DOMContentLoaded", handleDocumentLoad);

function handleDocumentLoad() {

    checkCookie();

    function getCookie(cname) {
        var name = cname + "=";
        var ca = document.cookie.split(';');
        for(var i = 0; i < ca.length; i++) {
            var c = ca[i];
            while (c.charAt(0) == ' ') {
                c = c.substring(1);
            }
            if (c.indexOf(name) == 0) {
                return c.substring(name.length, c.length);
            }
        }
        return "";
    }

    function checkCookie() {
        var nightmode = getCookie("lights");
        if (nightmode == "off") {
            lightsOff();
        } else {
            lightsOn();
        }
    }

    //Variables
    var offSwitch;
    var onSwitch;
    var style;

    //Functions
    function lightsOff() { /*This changes the background colour to black and makes text white*/

        offSwitch = document.getElementById("lightSwitchOff"); //Targets div with ID lightSwitchOff
        onSwitch = document.getElementById("lightSwitchOn"); //Targets div with ID lightSwitchOn
        style = document.getElementById("pageStyle"); //Targets stylsheet

        onSwitch.addEventListener("click", lightsOn); //When clicked this action is performed

        document.cookie = "lights = off;  expires = Fri, 31 Dec 9999 23:59:59 GMT";

        style.setAttribute('href', 'css/darkStyle.css');
        onSwitch.innerHTML = "Turn Off Night Mode";
        onSwitch.style.display = "inline";
        offSwitch.style.display = "none";
    }

    function lightsOn() { /*This changes the background colour to a white and makes text black*/

        offSwitch = document.getElementById("lightSwitchOff"); //Targets div with ID lightSwitchOff
        onSwitch = document.getElementById("lightSwitchOn"); //Targets div with ID lightSwitchOn
        style = document.getElementById("pageStyle"); //Targets stylsheet

        offSwitch.addEventListener("click", lightsOff); //When clicked this action is performed

        document.cookie = "lights = on;  expires = Fri, 31 Dec 9999 23:59:59 GMT";

        style.setAttribute('href', 'css/lightStyle.css');
        offSwitch.innerHTML = "Turn On Night Mode";
        onSwitch.style.display = "none";
        offSwitch.style.display = "inline";
    }
}
@charset "utf-8"

@font-face {
    font-family: 'Roboto', sans-serif;
}
/*Layout for phones and tablets e.g. iPhone 5 and iPad*/

    /*webpage fades in*/
    html {
        animation: fadein 2s;
        position: relative;
        min-height: 100%;
    }

    /*animation*/
    @keyframes fadein {
        from { opacity: 0; }
        to { opacity: 1; }
    }

    /*main colour settings for page*/
    body {
        color: #1C1C1C;
        font-family: 'Roboto';
        background-color: #FFF;
        margin: 0 0 100px;
        padding: 25px;
    }

    /*List in nav*/
    li {
        list-style-type: none;
        display: inline;
        font-size: 1.5em;
    }

    /*Navigation*/
    nav {
        max-width: 100%;
        max-height: 100%;
        border: solid 1px #1C1C1C;
        border-radius: 10px;
        padding-top: 10px;
        padding-bottom: 10px;
    }

    ol {
        text-align: center;
        margin-right: 6em;
    }

    /*Links*/
    a, a:link, a:visited, a:hover {
        color: #1C1C1C;
        text-decoration: none;
    }

    /*Main heading*/
    h1 {
        font-size: 3em;
        max-width: 100%;
        max-height: 100%;
        position: relative;
        margin: 0 auto;
        margin-left: -2.5em;
        text-align: center;
    }

    /*Secondary heading*/
    h2 {
        font-size: 2.75em;
        max-width: 100%;
        max-height: 100%;
    }

    /*Tertiary heading*/
    h3 {
        font-size: 2.5em;
        max-width: 100%;
        max-height: 100%;
    }

    /*Text*/
    p {
        font-size: 0.75em;
    }

    /*Images*/
    img {
        max-width: 100%;
        max-height: 100%;
        border: solid 0px #1C1C1C;
        border-radius: 5px;
        margin: 0 auto;
        text-align: center;
    }

    label {
        display: block;
        margin-bottom: 1em;
        margin-top: 1em;
    }

    textarea {
        max-width: 100%;
        max-height: 100%;
        width: 300px;
        height: 200px;
        margin-bottom: 10px;
        padding: 10px;
        border: solid 1px #1C1C1C;
        border-radius: 2px;
        resize: none;
    }

    input {
        border: solid 1px #1C1C1C;
        border-radius: 2px;
        padding: 5px;
    }

    #logo {
        float: left;
        margin-left: 10px;
        margin-top: 5px;
        max-height: 100px;
        max-width: 100px;
        border: 0px;
    }

    #enter {
        margin-top: 5em;
        margin-bottom: 5em;
        margin-left: 3em;
        max-width: 90%;
        max-height: 90%;
    }

    video {
        max-width: 80%;
        margin: 0 auto;
        display: block;
    }

    /*Footer*/
    footer {
        position: absolute;
        left: 0;
        bottom: 0;
        height: 100px;
        width: 100%;
        overflow: hidden;
        text-align: center;
    }

    /*Main Body*/
    #mainContent {
        padding: 10px;
        border: solid 1px #1C1C1C;
        border-radius: 5px;
        margin-top: 10px;
        margin-bottom: 10px;
    }

    #mainContent img {
        display: inline;
        max-width: 375px;
        max-height: 375px;
        float: right;
        margin-right: 2em;
        margin-left: 2em;
    }

    /*Light Switches*/
    #lightSwitchOff, #lightSwitchOn {
        display: none;
        margin-top: 10px;
    }


/*Layout for device with a min-width of 1024px*/
@media only screen and (min-width: 1024px) {

    p, li {
        font-size: 1.5em;
    }

    h1 {
        font-size: 2.75em;
    }

    h2 {
        font-size: 2.25em;
    }

    h3 {
        font-size: 2em;
    }

    #enter {
        max-width: 60%;
        max-height: 60%;
    }

    #lightSwitchOff, #lightSwitchOn {
        display: inline;
    }
}

/*Layout for desktop with a min-width of 1280px (720p HD)*/
@media only screen and (min-width: 1280px) {

    li {
        font-size: 1.5em;
    }

    h1 {
        font-size: 3em;
    }

    h2 {
        font-size: 2.25em;
    }

    #enter {
        max-width: 40%;
        max-height: 40%;
    }
}

/*Layout for desktop with a min-width of 1920px (1080p HD)*/
@media only screen and (min-width: 1920px) {

    p {
        font-size: 1.25em;
    }

    #enter {
        max-width: 40%;
        max-height: 40%;
    }
}

/*Layout for desktop with a min-width of 200px e.g. iMac*/
@media only screen and (min-width: 2000px) {

    p {
        font-size: 2em;
    }
}
.
<!DOCTYPE html>


<html lang="en">

    <head>

    <meta charset="utf-8"/>

    <title>Ryan Simms</title>

    <!-- ****** faviconit.com favicons ****** -->
    <link rel="shortcut icon" href="images/favicon.ico">
    <!-- ****** faviconit.com favicons ****** -->

    <link id ="pageStyle" rel="stylesheet" href="css/lightStyle.css" type="text/css"> <!-- Loads Default Stylesheet -->

    <style>
    @import url('https://fonts.googleapis.com/css?family=Roboto'); <!-- Custom Font -->
    </style>

    </head>

    <body>
    <script src="scripts/lightSwitch-contact.js"></script> <!-- Loads LightSwitch Script -->

    <ol>
    <a href="cover-letter-and-cv"><img alt="Enter" id="enter" title="Click to Enter" src="images/logo.jpg"></img></a>
    </ol>

    <footer>
      Website Created by Ryan Simms <br>
      <button type="button" id="lightSwitchOff"></button>
      <button type="button" id="lightSwitchOn"></button>
    </footer> <!-- Closes Footer -->

    </body>

</html>

\$\endgroup\$

2 Answers 2

Reset to default
2
\$\begingroup\$

The reason your page is initially loading without styles is because you programmed it to wait for he DOM to load before loading the styles :P

document.addEventListener ("DOMContentLoaded", handleDocumentLoad);

Your functions are small but, really, there's too much happening in them. Your function should only have to change the stylesheet. This type of stuff should be done in the stylesheet itself rather than with js:

  • onSwitch.style.display = "inline"; Remove theses lines of JS that are hiding and showing the buttons and do it with CSS in the appropriate stylesheet instead. eg, the lightsOff stylesheet should hide the lightsOff button and show the lightsOn button and the lightsOn stylesheet should do the opposite.
  • Don't use JS to change the innerHTML every time the function is run because that requires the DOM to be loaded, which slows everything down. This can be done in the HTML and doesn't need to be changed with JS offSwitch.innerHTML = "Turn On Night Mode"; Just put the text in the button and leave it alone: <button type="button" id="lightSwitchOff">Turn On Night Mode</button>

Once you make these changes your JS only needs a single element from the DOM: the stylesheet. As long as you put the script immediately below the stylesheet you don't need to wait for the DOM to load.

Here's the re-write:

function handleDocumentLoad() {
  var style = document.getElementById("pageStyle")

  checkCookie();

  function getCookie(cname) {
    var name = cname + "=";
    var ca = document.cookie.split(';');
    for (var i = 0; i < ca.length; i++) {
      var c = ca[i];
      while (c.charAt(0) == ' ') {
        c = c.substring(1);
      }
      if (c.indexOf(name) === 0) {
        return c.substring(name.length, c.length);
      }
    }
    return ""; 
  }

  function checkCookie() {
    var nightmode = getCookie("lights");
    if (nightmode == "off") {
      lightsOff();
    } else {
      lightsOn();
    }
  }

  function lightsOff() {
    document.cookie = "lights = off;  expires = Fri, 31 Dec 9999 23:59:59 GMT";
    style.setAttribute('href', 'darkStyle.css');
  }

  function lightsOn() {
    document.cookie = "lights = on;  expires = Fri, 31 Dec 9999 23:59:59 GMT";
    style.setAttribute('href', 'lightStyle.css');
  }

  document.addEventListener("DOMContentLoaded", function() {
    document.getElementById("lightSwitchOff").addEventListener("click", lightsOn);
    document.getElementById("lightSwitchOn").addEventListener("click", lightsOff);
  });

}

handleDocumentLoad();

Change your HTML to:

<button type="button" id="lightSwitchOff">Turn Off Night Mode</button>
<button type="button" id="lightSwitchOn">Turn On Night Mode</button>

Add these lines to lightStyle.css

#lightSwitchOff{ display:inline; }
#lightSwitchOn{ display:none; }

Add these lines to darkStyle.css

#lightSwitchOff{ display:none; }
#lightSwitchOn{ display:inline; }
\$\endgroup\$
15
  • \$\begingroup\$ Thank you for the help, the reason why I use the DOMContentLoaded is because I was taught that way in university (college). I'm still learning about website design more and more as I am more of a video games programmer. I knew I was repeating myself on certain lines of code but I wasn't so sure what else would work. I just got it to work and then would deal with the little annoying parts after. Thanks for your help! \$\endgroup\$ Commented Dec 13, 2017 at 19:42
  • \$\begingroup\$ Oh actually the code you gave doesn't work. \$\endgroup\$ Commented Dec 13, 2017 at 20:03
  • \$\begingroup\$ @Ryan - Can you make a plunkr duplicating the problem? It was intended as general advice, not code that you should copy/paste, but if you can duplicate the problem for me I'll be happy to update my answer. \$\endgroup\$ Commented Dec 13, 2017 at 20:06
  • \$\begingroup\$ I made a Plunkr using your code, I striped the HTML down to the basics so it's just the button for now: embed.plnkr.co/sndjHszYLo7QOX55Fj66 \$\endgroup\$ Commented Dec 13, 2017 at 23:28
  • 1
    \$\begingroup\$ Hey, sorry if I made you mad or anything. I never intended to do so, your help has been really appreciated and it really helped me learn more about JavaScript. I think I have found a solution and it seems to be working for me now. \$\endgroup\$ Commented Dec 14, 2017 at 21:30
1
\$\begingroup\$

I agree with the advice from I wrestled a bear once - especially about the DOM queries each time the function is run. If you did need to alter DOM elements when a function is run, it is best to store a reference to any such DOM elements in a variable when the DOM is ready and use that variable whenever the function is run.

In your original code, these three lines appear to exist in both functions (i.e. lightsOff() and lightsOn()). If you were to keep those variables, those assignment lines can be moved out to the handleDocumentLoad() function, since they are the same in both functions. This fits the Don't Repeat Yourself (i.e. D.R.Y.) principle.

offSwitch = document.getElementById("lightSwitchOff"); //Targets div with ID lightSwitchOff
onSwitch = document.getElementById("lightSwitchOn"); //Targets div with ID lightSwitchOn
style = document.getElementById("pageStyle"); //Targets stylsheet

For more tips like this, check out this article about optimizing Javascript. I know it is a few years old but still quite relevant (for now).

\$\endgroup\$
2
  • 2
    \$\begingroup\$ +1 for the article... the quotes are great.. and where's my hats invitation?? i'm jelly. \$\endgroup\$ Commented Dec 13, 2017 at 17:01
  • 1
    \$\begingroup\$ Thanks - come check out Winter Bash! \$\endgroup\$ Commented Dec 13, 2017 at 19:03

You must log in to answer this question.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.