TransWikia.com

CSS Alphabetizer

Code Review Asked on December 2, 2021

In another Code Review, it was recommended that I alphabetize my CSS. So I made a tool to do this quickly in JavaScript.

Because CSS has two levels (selector on the outside, parameters on the inside), and they both need to be alphabetized, alphabetizing it is not as easy as just doing array.sort(). A custom tool is needed.

Feel free to comment on anything JavaScript. Here’s my code.

"use strict";

class CSSAlphabetizer {
    alphabetize(s) {
        let lines, blocks;
        
        // TODO: this._validate();
        // TODO: this._beautify(); using regex and replace of { ; }
        
        lines = this._makeLinesArray(s);
        blocks = this._sortSelectors(lines);
        blocks = this._sortParameters(blocks);
        
        s = this._makeString(blocks);
        s = this._addBlankLineBetweenBlocks(s);
        s = s.trim();
        
        return s;
    }
    
    _addBlankLineBetweenBlocks(s) {
        return s.replace(/}n([^n])/, "}nn$1");
    }
    
    _makeString(blocks) {
        // blocks => lines
        let lines = [];
        for ( let block of blocks ) {
            let lines2 = block[1];
            for ( let line of lines2 ) {
                lines.push(line);
            }
        }
        
        // lines => string
        let s = "";
        for ( let line of lines ) {
            s += line[1] + "n";
        }
        s = s.slice(0, s.length-1);
        return s;
    }
    
    _sortSelectors(lines) {
        /** [parameterTrimmed, [line, line, line, line]] */
        let blocks = [];
        /** [line, line, line, line] **/
        let buffer = [];
        let lineNumTracker = 0;
        let parameterTrimmed = "";
        let len = lines.length;
        
        for ( let i = 0; i < len; i++ ) {
            let line = lines[i];
            let lineNum = line[2];
            
            if ( ! parameterTrimmed && line[0].includes("{") ) {
                parameterTrimmed = line[0];
            }
            
            if ( lineNum !== lineNumTracker ) {
                lineNumTracker++;
                blocks.push([parameterTrimmed, buffer]);
                buffer = [];
                parameterTrimmed = "";
            }
            
            buffer.push(line);
            
            // Last line. Finish the block.
            if ( i === len-1 ) {
                lineNumTracker++;
                blocks.push([parameterTrimmed, buffer]);
                buffer = [];
                parameterTrimmed = "";
            }
        }
        
        blocks = blocks.sort();
        
        return blocks;
    }
    
    _sortParameters(blocks) {
        for ( let key in blocks ) {
            let lines = blocks[key][1];
            let lineBuffer = [];
            let sortBuffer = [];
            
            for ( let line of lines ) {
                let isParameter = line[3];
                
                if ( isParameter ) {
                    sortBuffer.push(line);
                } else {
                    if ( sortBuffer ) {
                        sortBuffer = sortBuffer.sort();
                        lineBuffer = lineBuffer.concat(sortBuffer);
                    }
                    
                    lineBuffer.push(line);
                }
            }
            
            blocks[key][1] = lineBuffer;
        }
        console.log(blocks);
        return blocks;
    }
    
    /** Makes an array of arrays. [trimmed, notTrimmed, blockNum, isParameter]. Having unique block numbers will be important later, when we are alphabetizing things. */
    _makeLinesArray(s) {
        // TODO: refactor to use associative array, more readable code, and less likely to have bugs if later we want to add/change/delete fields
        let lines = s.split("n");
        let blockNum = 0;
        let isParameter = false;
        
        for ( let key in lines ) {
            let value = lines[key];
            
            if ( value.includes("}") ) {
                isParameter = false;
            }
            
            lines[key] = [
                // Trimmed line in first spot. Important for sorting.
                value.trim(),
                value,
                blockNum,
                isParameter,
            ];
            
            // When } is found, increment the block number. This keeps comment lines above block grouped with that block.
            if ( value.includes("}") ) {
                blockNum++;
            }
            
            if ( value.includes("{") ) {
                isParameter = true;
            }
        }
        
        console.log(lines);
        
        return lines;
    }
}

window.addEventListener('DOMContentLoaded', (e) => {
    let css = document.getElementById('css');
    let alphabetize = document.getElementById('alphabetize');
    
    // TODO: Combo box that loads tests.
    
    alphabetize.addEventListener('click', function(e) {
        let alphabetizer = new CSSAlphabetizer();
        css.value = alphabetizer.alphabetize(css.value);
    });
});
body {
    margin: 1em;
    width: 95%;
    max-width: 700px;
    font-family: sans-serif;
}

textarea {
    width: 100%;
    height: 360px;
    tab-size: 4;
}
<!DOCTYPE html>

<html lang="en-us">

<head>
    <title>CSS Alphabetizer</title>
    <link rel="stylesheet" href="style.css" />
    <script type="module" src="css-alphabetizer.js"></script>
</head>

<body>
    <h1>
    CSS Alphabetizer
    </h1>
    
    <p>
    Beautify your CSS. Sort your selectors alphabetically (which groups together #id's, .classes, and then elements), and sort your parameters alphabetically too. Parameters are nested within braces, so those aren't easy to sort with a normal A-Z sort.
    </p>
    
    <p>
    <textarea id="css">body {
    background-color: #999999;
    margin-bottom: 0;
}

#page-container {
    background-color: #DFDFDF;
    width: 1150px;
    margin: 12px auto 0 auto;
}

#page-container2 {
    padding: 12px 12px 0 12px;
}

header {
    background-color: white;
}

.category {
    display: inline-block;
    padding: 18px 25px;
    font-family: sans-serif;
}

.category:hover {
    background-color: #059BD8;
}</textarea>
    </p>
    
    <p>
    <button id="alphabetize">Alphabetize</button>
    </p>
    
    <p>
    Want to report a bug or request a feature? <a href="https://github.com/RedDragonWebDesign/css-alphabetizer/issues">Create an issue</a> on our GitHub.
    </p>
</body>

</html>

2 Answers

From a long review;

  • You should use a beautifier it will turn if ( i === len-1 ) to if (i === len-1)

  • This could really use an IFFE to hide your worker functions and expose 1 actual function, the class seems overkill.

  • You can club together some statements

      blocks = this._sortSelectors(lines);
      blocks = this._sortParameters(blocks);
    

    can become

      blocks = sortParameters(sortSelectors(lines));
    

    and

      s = this._makeString(blocks);
      s = this._addBlankLineBetweenBlocks(s);
      s = s.trim();
    

    can become

      s = this.separateBlocks(makeString(blocks)).trim();
    
  • This looks write you wrote your own String.join()

      // lines => string
      let s = "";
      for ( let line of lines ) {
          s += line[1] + "n";
      }
      s = s.slice(0, s.length-1);
      return s;
    

    you could just

      return lines.map(line=>line[1]).join('n');
    
  • Always consider FP when dealing with list;

      let lines = [];
      for ( let block of blocks ) {
          let lines2 = block[1];
          for ( let line of lines2 ) {
              lines.push(line);
          }
      }
    

    could be

      const lines = blocks.map(block => block[1]).flat(); 
    
  • Minor nitpicking on this;

              // Trimmed line in first spot. Important for sorting.
              value.trim(),
    

    You could have written a custom sort function that does the trim()

  • Dont do console.log in final code

  • let vs. const always requires some contemplation

    • let value = lines[key]; should be const value = lines[key];
    • let isParameter = line[3]; should be a const
    • let lines = s.split("n"); should be a const
  • Potential Parsing Problems

    • value.includes("}" would go funky with some comments /* } */
    • Also, value.includes("{") of course
  • If you like ternary operators then

          if ( value.includes("{") ) {
              isParameter = true;
          }
    

    could be

          isParameter = value.includes("{") ? true : isParameter;
    

    however I think your version reads better

  • The html could use a <meta charset="UTF-8">

  • if ( sortBuffer ) { always executes, you probably meant to go for if ( sortBuffer.length ) {, personally I dropped the if statement altogether for elegance, the code works either way

The below is my counter proposal, my FP skills were not good enough to convert sortSelectors and sortParameters but I am sure there is a way to make these functions cleaner;

/*jshint esnext:true */

const styleSorter = (function createStyleSorter(){
  "use strict";
  
  // TODO: this._validate();
  // TODO: this._beautify(); using regex and replace of { ; }
  function alphabetize(s){
    
    const lines = makeLines(s + 'n');
    const blocks = sortParameters(sortSelectors(lines));

    return separateBlocks(makeString(blocks)).trim();        
  }
  
  //TODO: Always document your regex expressions
  function separateBlocks(s) {
    return s.replace(/}n([^n])/, "}nn$1");
  }  
  
  function makeString(blocks){
    //get 2nd element of each block, flatten, get 2nd element again
    const lines = blocks.map(block => block[1]).flat().map(fields => fields[1]);
    return lines.join('n');
  }
    
  //Makes an array of arrays. [trimmed, notTrimmed, blockNum, isParameter].
  //Having unique block numbers will be important later, when we sort
  //TODO: refactor with associative array <- Not sure that will help much?
  function makeLines(s) {
    let blockNum = 0;
    let isParameter = false;
        
    return s.split("n").map((value, key) => {
          
      if ( value.includes("}") ) {
        isParameter = false;
      }
          
      //First value is trimmed for sorting
      const line = [value.trim(), value, blockNum, isParameter];
          
      if ( value.includes("}") ) {
        blockNum++;
      }
            
      if ( value.includes("{") ) {
        isParameter = true;
      }          
          
      return line;
    });
  }
  
  function sortSelectors(lines) {
    /** [parameterTrimmed, [line, line, line, line]] */
    let blocks = [];
    /** [line, line, line, line] **/
    let buffer = [];
    let lineNumTracker = 0;
    let parameterTrimmed = "";
    let len = lines.length;
        
    for ( let i = 0; i < len; i++ ) {
      let line = lines[i];
      let lineNum = line[2];
            
      if (!parameterTrimmed && line[0].includes("{") ) {
        parameterTrimmed = line[0];
      }
            
      if (lineNum !== lineNumTracker) {
        lineNumTracker++;
        blocks.push([parameterTrimmed, buffer]);
        buffer = [];
        parameterTrimmed = "";
      }
            
      buffer.push(line);
    }
        
    return blocks.sort();
  }
  
  function sortParameters(blocks) {
    const IS_PARAMETER = 3;
    for ( let key in blocks ) {
      let lines = blocks[key][1];
      let lineBuffer = [];
      let sortBuffer = [];
            
      for (let line of lines) {
                
        if (line[IS_PARAMETER]) {
          sortBuffer.push(line);
        } else {
          lineBuffer = lineBuffer.concat(sortBuffer.sort());
          lineBuffer.push(line);                    
        }
        
        blocks[key][1] = lineBuffer;
      }
      return blocks;
    }
  }
  
  return alphabetize;  
})();

window.addEventListener('DOMContentLoaded', (e) => {
  let css = document.getElementById('css');
  let alphabetize = document.getElementById('alphabetize');
    
  // TODO: Combo box that loads tests.
  alphabetize.addEventListener('click', function(e) {
    css.value = styleSorter(css.value);
  });
});
body {
    margin: 1em;
    width: 95%;
    max-width: 700px;
    font-family: sans-serif;
}

textarea {
    width: 100%;
    height: 360px;
    tab-size: 4;
}
<!DOCTYPE html>

<html lang="en-us">

<head>
    <meta charset="UTF-8">
    <title>CSS Alphabetizer</title>
    <link rel="stylesheet" href="style.css" />
    <script type="module" src="css-alphabetizer.js"></script>
</head>

<body>
    <h1>
    CSS Alphabetizer
    </h1>
    
    <p>
    Beautify your CSS. Sort your selectors alphabetically (which groups together #id's, .classes, and then elements), and sort your parameters alphabetically too. Parameters are nested within braces, so those aren't easy to sort with a normal A-Z sort.
    </p>
    
    <p>
    <textarea id="css">body {
    background-color: #999999;
    margin-bottom: 0;
}

#page-container {
    background-color: #DFDFDF;
    width: 1150px;
    margin: 12px auto 0 auto;
}

#page-container2 {
    padding: 12px 12px 0 12px;
}

header {
    background-color: white;
}

.category {
    display: inline-block;
    padding: 18px 25px;
    font-family: sans-serif;
}

.category:hover {
    background-color: #059BD8;
}</textarea>
    </p>
    
    <p>
    <button id="alphabetize">Alphabetize</button>
    </p>
    
    <p>
    Want to report a bug or request a feature? <a href="https://github.com/RedDragonWebDesign/css-alphabetizer/issues">Create an issue</a> on our GitHub.
    </p>
</body>

</html>

Answered by konijn on December 2, 2021

To be honest, I didn't read all of the code. I gave up half way through. I believe this is due to two points:

  • You are not using the established CSS terms, e.g. "block" instead of "rule" and "parameter" instead of "declaration".

  • Most of the comments confused me more than they helped.

Additionally the parsing is too simple. It will choke on unexpected braces, for example content: "}", and on group rules such as media queries.

Answered by RoToRa on December 2, 2021

Add your own answers!

Ask a Question

Get help from others!

© 2024 TransWikia.com. All rights reserved. Sites we Love: PCI Database, UKBizDB, Menu Kuliner, Sharing RPP