Skip to main content
Bounty Ended with 150 reputation awarded by CommunityBot
Back to basics
Source Link
T145
  • 3.1k
  • 2
  • 22
  • 44
  1. 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."
  2. 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.
  3. 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.
  4. I'd swap function() { iterations with () => { just because it's a bit cleaner.
  5. Your resetter event listener (the only significant logic here) can be simplified by checking if the editor value is default first, then placing the first block in an else-if with confirm as the condition.
  6. Separate your JS into a file and link it like so: <script type="text/javascript" src="editor.js"></script>
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();
  }
});
  1. Separate your JS into a file and link it like so: <script type="text/javascript" src="editor.js"></script>
  1. 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."
  2. 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.
  3. 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.
  4. I'd swap function() { iterations with () => { just because it's a bit cleaner.
  5. 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();
  }
});
  1. Separate your JS into a file and link it like so: <script type="text/javascript" src="editor.js"></script>
  1. 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."
  2. 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.
  3. 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.
  4. I'd swap function() { iterations with () => { just because it's a bit cleaner.
  5. Your resetter event listener (the only significant logic here) can be simplified by checking if the editor value is default first, then placing the first block in an else-if with confirm as the condition.
  6. Separate your JS into a file and link it like so: <script type="text/javascript" src="editor.js"></script>
Better CSS explanation
Source Link
T145
  • 3.1k
  • 2
  • 22
  • 44
  1. 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.
  2. While the charset is also case insensitive, it seems to be modern convention to leave it lowercase, as is in the Bootstrap examples.
  3. Separate your CSS into a file and link it like so: <link rel="stylesheet" href="styles.css">
  4. You favor ID selectors over class selectors, which IMO is a missed opprotunity here. Rather than having to specify a unique ID for each custom elementInstead of repeatedly specifying elements, you could instead usecreate a single CSS class acrossand assign it to the elements that useyou want the same CSS to be applied to. The span definitionelement content for example is the primary offenderassigned to 7 other unique elements, which is not a good practice.
  5. 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:
  1. 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.
  2. While the charset is also case insensitive, it seems to be modern convention to leave it lowercase, as is in the Bootstrap examples.
  3. Separate your CSS into a file and link it like so: <link rel="stylesheet" href="styles.css">
  4. You favor ID selectors over class selectors, which IMO is a missed opprotunity here. Rather than having to specify a unique ID for each custom element, you could instead use a single class across the elements that use the same CSS. The span definition is the primary offender.
  5. 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:
  1. 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.
  2. While the charset is also case insensitive, it seems to be modern convention to leave it lowercase, as is in the Bootstrap examples.
  3. Separate your CSS into a file and link it like so: <link rel="stylesheet" href="styles.css">
  4. 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.
  5. 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:
Added resetter simplification
Source Link
T145
  • 3.1k
  • 2
  • 22
  • 44
  1. 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."
  2. 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.
  3. 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.
  4. I'd swap function() { iterations with () => { just because it's a bit cleaner.
  5. Your resetter event listener (the only significant logic here) can be greatly simplified by checking if the editor value is default first, then placing the first block in an else-if with confirm as the condition.
  6. Separate your JS into a file and link it like so: <script type="text/javascript" src="editor.js"></script>
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();
  }
});
  1. Separate your JS into a file and link it like so: <script type="text/javascript" src="editor.js"></script>
  1. 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."
  2. 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.
  3. 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.
  4. I'd swap function() { iterations with () => { just because it's a bit cleaner.
  5. Your resetter event listener (the only significant logic here) can be greatly simplified by checking if the editor value is default first, then placing the first block in an else-if with confirm as the condition.
  6. Separate your JS into a file and link it like so: <script type="text/javascript" src="editor.js"></script>
  1. 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."
  2. 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.
  3. 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.
  4. I'd swap function() { iterations with () => { just because it's a bit cleaner.
  5. 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();
  }
});
  1. Separate your JS into a file and link it like so: <script type="text/javascript" src="editor.js"></script>
Source Link
T145
  • 3.1k
  • 2
  • 22
  • 44
Loading