Return to Revisions

3 of 4

Better CSS explanation

edited May 14, 2023 at 17:37

3.1k
2
22
44

I'll be reviewing the code in your GitLab repository as it has this code in one file:

index.html

<!DOCTYPE html>
<html lang="en">

<head>
  <meta charset="UTF-8">
  <meta name="viewport" content="width=device-width, initial-scale=1">
  <meta name="description" content="Edit your HTML, CSS, and JavaScript code and monitor the instant live preview.">
  <title>HTML Editor: online HTML editor with real-time preview</title>
  <link rel="icon" href="favicon.ico">
  <style>
    html,
    body {
      margin: 0;
      padding: 0;
      height: 100%;
    }

    body {
      display: flex;
      flex-direction: column;
    }

    header,
    footer:not([hidden]) {
      display: flex;
      flex-wrap: wrap;
      align-items: center;
      gap: 5px;
      padding: 5px;
    }

    header {
      background: linear-gradient(#FFF, #CCC);
    }

    label,
    #downloader,
    select,
    #resetter,
    #selector,
    #fileChooser,
    output,
    span {
      font: bold 11px Arial;
      color: #333;
    }

    [type="checkbox"] {
      margin: 0 5px 0 0;
    }

    [for="fontSizer"] {
      margin-left: 5px;
    }

    select,
    button,
    #resizer {
      margin: 0;
    }

    #fileChooser {
      margin: 0 auto 0 0;
    }

    #resizer,
    iframe {
      padding: 0;
    }

    output {
      margin-right: 5px;
      font-family: monospace;
    }

    #footerToggler {
      width: 16px;
      height: 16px;
      border: 1px solid #666;
      border-bottom-width: 5px;
      padding: 0;
      background: transparent;
    }

    #footerToggler.on {
      border-color: #333;
      background: #FFF;
    }

    main {
      flex: 1;
      display: flex;
    }

    main.horizontal {
      flex-direction: column;
    }

    div {
      flex: 0px;
      min-width: 0;
      min-height: 0;
    }

    #viewerWrapper {
      border-left: 5px solid #CCC;
    }

    main.horizontal #viewerWrapper {
      border-left: 0;
      border-top: 5px solid #CCC;
    }

    div * {
      display: block;
      width: 100%;
      height: 100%;
      margin: 0;
      border: 0;
      background: #FFF;
    }

    textarea {
      box-sizing: border-box;
      padding: 5px;
      outline: 0;
      resize: none;
      font-size: 14px;
      color: #333;
    }

    textarea.dark {
      background: #333;
      color: #FFF;
    }

    footer {
      background: linear-gradient(#CCC, #FFF);
    }

    img {
      display: block;
    }

    #copier {
      border: 0;
      padding: 0;
      background: transparent;
      cursor: pointer;
    }

    address {
      margin-left: auto;
      font: italic 16px 'Times New Roman';
      color: #333;
    }

    address a {
      color: inherit;
    }
  </style>
</head>

<body>
  <header>
    <label for="runner">Run</label>
    <input type="checkbox" id="runner" checked>
    <a href="" download="template.html" title="Download the HTML document" id="downloader">Download</a>
    <label for="fontSizer">Font size</label>
    <select id="fontSizer">
      <option>12</option>
      <option>13</option>
      <option selected>14</option>
      <option>15</option>
      <option>16</option>
      <option>17</option>
      <option>18</option>
      <option>19</option>
      <option>20</option>
    </select>
    <button type="button" id="resetter">Reset</button>
    <button type="button" id="selector">Select</button>
    <input type="file" accept="text/html" id="fileChooser">
    <label for="resizer">Editor size</label>
    <input type="range" id="resizer">
    <output for="resizer" id="indicator">0.50</output>
    <label for="viewsToggler">Horizontal view</label>
    <input type="checkbox" id="viewsToggler">
    <label for="themesToggler">Dark theme</label>
    <input type="checkbox" id="themesToggler">
    <button type="button" title="Toggle footer" id="footerToggler"></button>
  </header>
  <main id="main">
    <div id="editorWrapper">
      <textarea spellcheck="false" id="editor"><!DOCTYPE html>
<html lang="en">
<head>
  <title>HTML Document Template</title>
  <style>
    p {
      font-family: Arial;
    }
  </style>
</head>
<body>
  <p>Hello, world!</p>
  <script>
    console.log(document.querySelector('p').textContent);
  </script>
</body>
</html></textarea>
    </div>
    <div id="viewerWrapper">
      <iframe id="viewer"></iframe>
    </div>
  </main>
  <footer id="footer" hidden>
    <span>Share</span>
    <a href="https://twitter.com/intent/tweet?text=HTML%20Editor%3A%20online%20HTML%20editor%20with%20real-time%20preview&url=https%3A%2F%2Fhtmleditor.gitlab.io" target="_blank"><img src="images/twitter.svg" width="16" height="16" alt="Twitter"></a>
    <a href="https://www.facebook.com/sharer.php?u=https%3A%2F%2Fhtmleditor.gitlab.io&t=HTML%20Editor%3A%20online%20HTML%20editor%20with%20real-time%20preview" target="_blank"><img src="images/facebook.svg" width="16" height="16" alt="Facebook"></a>
    <a href="https://www.linkedin.com/sharing/share-offsite/?url=https%3A%2F%2Fhtmleditor.gitlab.io" target="_blank"><img src="images/linkedin.svg" width="16" height="16" alt="LinkedIn"></a>
    <a href="mailto:?subject=HTML%20Editor%3A%20online%20HTML%20editor%20with%20real-time%20preview&body=https%3A%2F%2Fhtmleditor.gitlab.io" target="_blank"><img src="images/email.svg" width="16" height="16" alt="Email"></a>
    <button type="button" id="copier"><img src="images/link.svg" width="16" height="16" alt="Link"></button>
    <span id="notification" hidden>Copied!</span>
    <address><a href="https://codereview.stackexchange.com/questions/284918/html-editor-online-html-editor-with-real-time-preview" title="Code Review Stack Exchange">Feedback</a> | Created by <a href="https://mori.pages.dev" rel="author">Mori</a></address>
  </footer>
  <script>
    var runner = document.getElementById('runner'),
      editor = document.getElementById('editor'),
      downloader = document.getElementById('downloader'),
      fileChooser = document.getElementById('fileChooser');

    function preview() {
      if (runner.checked) {
        document.getElementById('viewer').srcdoc = editor.value;
      }
    }

    editor.addEventListener('input', preview);
    runner.addEventListener('change', preview);

    ['click', 'contextmenu'].forEach(event => downloader.addEventListener(event, function() {
      var blob = new Blob([editor.value], {type: 'text/html'});
      this.href = URL.createObjectURL(blob);
    }));

    document.getElementById('fontSizer').addEventListener('change', function() {
      editor.style.fontSize = this.value + 'px';
    });

    document.getElementById('resetter').addEventListener('click', function() {
      function resetFileChooserAndDownload() {
        fileChooser.value = '';
        downloader.download = 'template.html';
      }
      if (!editor.value || editor.value != editor.defaultValue && confirm('Your input will be lost.\nAre you sure you want to reset?')) {
        resetFileChooserAndDownload();
        editor.value = editor.defaultValue;
        preview();
      } else if (editor.value == editor.defaultValue) {
        resetFileChooserAndDownload();
      }
    });

    document.getElementById('selector').addEventListener('click', function() {
      editor.select();
    });

    fileChooser.addEventListener('change', async function() {
      var file = this.files[0];
      if (file) { // to ensure that there's a file to read so Chrome, for example, doesn't run this function when you cancel choosing a new file
        downloader.download = file.name;
        editor.value = await file.text();
        preview();
      }
    });

    document.getElementById('resizer').addEventListener('input', function() {
      var resizerVal = this.value;
      document.getElementById('editorWrapper').style.flexGrow = resizerVal;
      document.getElementById('viewerWrapper').style.flexGrow = 100 - resizerVal;
      document.getElementById('indicator').value = (resizerVal / 100).toFixed(2);
    });

    document.getElementById('viewsToggler').addEventListener('change', function() {
      document.getElementById('main').classList.toggle('horizontal');
    });

    document.getElementById('themesToggler').addEventListener('change', function() {
      editor.classList.toggle('dark');
    });

    document.getElementById('footerToggler').addEventListener('click', function() {
      this.classList.toggle('on');
      document.getElementById('footer').toggleAttribute('hidden');
    });

    document.getElementById('copier').addEventListener('click', function() {
      navigator.clipboard.writeText('https://htmleditor.gitlab.io');
      function toggleNotification() {
        document.getElementById('notification').toggleAttribute('hidden');
      }
      toggleNotification();
      setTimeout(toggleNotification, 1500);
    });

    window.addEventListener('beforeunload', function(event) {
      if (editor.value && editor.value != editor.defaultValue) {
        event.preventDefault();
        event.returnValue = '';
      }
    });

    preview();
  </script>
</body>

</html>

And start from the top.

The DOCTYPE being capitalized shows that you're still considering support for HTML4 or XHTML serializations. While it's case insensitive, the modern examples I've seen all leave it lowercase.
While the charset is also case insensitive, it seems to be modern convention to leave it lowercase, as is in the Bootstrap examples.
Separate your CSS into a file and link it like so: <link rel="stylesheet" href="styles.css">
You favor ID selectors over class selectors, which IMO is a missed opprotunity here. Instead of repeatedly specifying elements, create a single CSS class and assign it to the elements you want the CSS to be applied to. The span element content for example is assigned to 7 other unique elements, which is not a good practice.
The flex-direction, flex-wrap, align-items, and flex properties are all invalid as of CSS 3.0. Also 0px can just be 0. Here's the optimized CSS file:

styles.css

html,body {
    height:100%;
    margin:0;
    padding:0
}

header,footer:not([hidden]) {
    display:flex;
    padding:5px
}

header {
    background:linear-gradient(#FFF,#CCC)
}

label,#downloader,select,#resetter,#selector,#fileChooser,output,span {
    color:#333;
    font:bold 11px Arial
}

[type="checkbox"] {
    margin:0 5px 0 0
}

[for="fontSizer"] {
    margin-left:5px
}

select,button,#resizer {
    margin:0
}

#fileChooser {
    margin:0 auto 0 0
}

#resizer,iframe {
    padding:0
}

output {
    font-family:monospace;
    margin-right:5px
}

#footerToggler {
    background:transparent;
    border:1px solid #666;
    border-bottom-width:5px;
    height:16px;
    padding:0;
    width:16px
}

#footerToggler.on {
    background:#FFF;
    border-color:#333
}

div {
    min-height:0;
    min-width:0
}

#viewerWrapper {
    border-left:5px solid #CCC
}

main.horizontal #viewerWrapper {
    border-left:0;
    border-top:5px solid #CCC
}

div * {
    background:#FFF;
    border:0;
    display:block;
    height:100%;
    margin:0;
    width:100%
}

textarea {
    box-sizing:border-box;
    color:#333;
    font-size:14px;
    outline:0;
    padding:5px;
    resize:none
}

textarea.dark {
    background:#333;
    color:#FFF
}

footer {
    background:linear-gradient(#CCC,#FFF)
}

img {
    display:block
}

#copier {
    background:transparent;
    border:0;
    cursor:pointer;
    padding:0
}

address {
    color:#333;
    font:italic 16px 'Times New Roman';
    margin-left:auto
}

address a {
    color:inherit
}

body,main {
    display:flex
}

Onto the HTML and JS, the most terrifying aspect here is the fact that user-inputted JS can run in the browser. In security consideration terms, this could open a door to XSS attacks. Fortunately this project is raw HTML with no external dependencies and run client-side, so any modern web browser should give protections by default. However sanitizing any given HTML would be an excellent practice. I'd also take a look at the other "best practices."
It's unclear which JS version you're targeting, but assuming it's in the ballpark of ES2019 you should be using let over var so your variables are scoped to enclosed blocks.
Your preview function is effectively acting like a main function. Create a proper main function that registers each listener so your element selector variables can be scoped properly. This again ties into security.
I'd swap function() { iterations with () => { just because it's a bit cleaner.
Your resetter event listener (the only significant logic here) can be simplified:

document.getElementById('resetter').addEventListener('click', () => {
  fileChooser.value = '';
  downloader.download = 'template.html';

  if (editor.value != editor.defaultValue && confirm('Your input will be lost.\nAre you sure you want to reset?')) {
    editor.value = editor.defaultValue;
    preview();
  }
});

Separate your JS into a file and link it like so: <script type="text/javascript" src="editor.js"></script>

answered May 13, 2023 at 16:48

T145

3.1k
2
22
44