7
\$\begingroup\$

So my problem is this, I need to create a grid that in each cell there should be a 0 or a 1. These cells when clicked should toggle between 0 and 1. When hovered It should show the coordinates (ex: 1,5). Now the only way I got this to work was by creating three divs. One for the cell (which contains the number), one for the coordinates (this one is added dynamically) and another div (wrapper) that will on top of the other two and this one will have the event listeners. So basically I'm creating three divs for cell, now this work perfectly if its for a 10x10 grid, but when It gets to a more large (64x64) the browser starts to freeze.

This is how the HTML looks for a cell of the grid:

<div class="cell cellUnselected" id="cell_1_1" style="left: 0px; top: 0px;">0</div>
<div class="cellCoordinates cellCoordText" id="cell_1_1_coord" style="left: 0px; top: 0px;"></div>
<div class="cellWrapper" id="cell_1_1_wrapper" style="left: 0px; top: 0px;"></div>

I created a working fiddle please take a look:

http://jsfiddle.net/vicgonzalez25/Tfs2M/

The problem: Once the grid start getting to a bigger size (ex: 64x64) by creating these three divs the browser starts to freeze.

My question is: is there a more efficient way of doing this grid ? Maybe using a table ? Thanks

In order to reproduce:

HTML:

<div id="gridLayout" class="gridLayout">
    <div id="gridHeader">
        <h2>Aperture Configuration:</h2>
        Grid Size:
        <input id="rows" type="number" min="1" max="50" value="10" width="40" size="3" onChange="GRASP.start();">
        x
        <input id="cols" type="number" min="1" max="50" value="10" width="40" size="3" onChange="GRASP.start();">
    </div>
    <div id="grid" class="gridContainer"></div>

    <div id="matrixHeader" style="position:absolute">
        <h2>Auto Correlation:</h2>
    </div>
    <div id="matrix" class="autocorrMatrixContainer"></div>
</div>

Javascript:

(function(GRASP, $){
    var GRID_ROWS,
        GRID_COLS,
        GRID_ELEMENT,
        MATRIX_ROWS,
        MATRIX_COLS,
        MATRIXHEADER_ELEMENT,
        MATRIX_ELEMENT,
        A,C;

    GRASP.config = {
        gridContainer: "grid",
        matrixContainer: "matrix",
        matrixHeader: "matrixHeader"
    };

    GRASP.start = function(){
        GRID_ROWS = $("#rows").val();
        GRID_COLS = $("#cols").val();
        MATRIX_ROWS = GRID_ROWS * 2 - 1;
        MATRIX_COLS = GRID_COLS * 2 - 1;
        createGrid();
        createAutocorrelationMatrix();
    };

    function createGrid()
    {
        GRID_ELEMENT = $("#"+GRASP.config.gridContainer);
        GRID_ELEMENT.html(""); // Clear Grid ;)
        var coord;
        var cell; // Contains the 1 or 0 based upon the cell selection

        for(var i=1; i<=GRID_ROWS; i++){
            for(var j=1; j<=GRID_COLS; j++){
                coord = "" + i + "," + j;

                $(document.createElement("div"))
                    .addClass("cellWrapper")
                    .attr("alt", coord)
                    .css("left", parseInt((j-1) * 36, 10) + "px")
                    .css("top", parseInt((i-1) * 36, 10) + "px")
                    .width(36).height(36)
                    .data("row", i).data("col", j)
                    .appendTo("#"+GRASP.config.gridContainer)
                    .on("click", cellClick)
                    .on("mouseenter", {isMatrix: false}, cellMouseEnter)
                    .on("mouseleave", cellMouseLeave);

                $(document.createElement("div"))
                    .addClass("cell cellUnselected")
                    .attr("alt", coord)
                    .css("left", parseInt((j-1) * 36, 10) + "px")
                    .css("top", parseInt((i-1) * 36, 10) + "px")
                    .text("0")
                    .appendTo("#"+GRASP.config.gridContainer);
            }
        }

        GRID_ELEMENT.height(36 * GRID_ROWS);
        GRID_ELEMENT.width(36 * GRID_COLS);

    }

    function createAutocorrelationMatrix() {
        MATRIXHEADER_ELEMENT = $("#" + GRASP.config.matrixHeader);
        MATRIX_ELEMENT = $("#" + GRASP.config.matrixContainer);
        MATRIX_ELEMENT.html("");

        MATRIXHEADER_ELEMENT.css("top", parseInt(GRID_ELEMENT.offset().top + (GRID_ROWS * 36)) + "px");
        MATRIX_ELEMENT.css("top", parseInt(MATRIXHEADER_ELEMENT.offset().top + MATRIXHEADER_ELEMENT.height()) + "px");

        var cellSize = Math.ceil((GRID_ROWS * 36) / MATRIX_ROWS);
        var coord;

        for (var i=1;i<=MATRIX_ROWS;i++){
            for (var j=1;j<=MATRIX_COLS;j++){
                coord = "" + i + "," + j;
                $(document.createElement("div"))
                    .addClass("autocorrMatrixCellWrapper")
                    .attr("alt", coord)
                    .css("left", parseInt((j-1) * cellSize, 10) + "px")
                    .css("top", parseInt((i-1) * cellSize, 10) + "px")
                    .data("row", i).data("col", j)
                    .appendTo("#"+GRASP.config.matrixContainer)
                    .on("mouseenter", {isMatrix: true}, cellMouseEnter)
                    .on("mouseleave", cellMouseLeave);

                $(document.createElement("div"))
                    .addClass("autocorrMatrixCell autocorrMatrixCellUnselected")
                    .attr("alt", coord)
                    .css("left", parseInt((j-1) * cellSize, 10) + "px")
                    .css("top", parseInt((i-1) * cellSize, 10) + "px")
                    .appendTo("#"+GRASP.config.matrixContainer);
            }
        }

        MATRIX_ELEMENT.height(36 * GRID_ROWS);
        MATRIX_ELEMENT.width(36 * GRID_COLS);
    }

    function cellClick(){
        var cell = $(this).next();

        if(cell.text() == "0"){
            cell.text("1");
        } else {
            cell.text("0");
        }
    }

    function cellMouseEnter(e){
        var i = $(this).data("row");
        var j = $(this).data("col");

        var x = e.data.isMatrix ? Math.ceil((GRID_ROWS * 36) / MATRIX_ROWS) : 36;

        var div = $(document.createElement("div"))
            .addClass("cellCoordinates cellCoordText")
            .css("left", parseInt((j-1) * x, 10) + "px")
            .css("top", parseInt((i-1) * x, 10) + "px")
            .text(i + ", " + j);

        $(this).before(div);
    }

    function cellMouseLeave(){
        $(this).prev().remove();
    }

}(window.GRASP = window.GRASP || {}, jQuery));

$(document).ready(function(){
    GRASP.start();
});

CSS:

.gridContainer {
/*  width: inherit; */
/*  float: left; */
    position: relative;
    top: 10px;
    padding: 0 0 0 0;
    display: block;
    background: none;
    font-family: Arial, Helvetica, Verdana, sans-serif;
}

.cell {
    width: 36px;
    height: 36px;
    position: absolute;
    z-index: 0;

/*
    font-size: 16pt;
*/
    font-size: x-large;
    font-weight: normal;
    font-style: normal;
    color: #888888;
    text-align: center;
    display: table-cell;
    vertical-align: middle;
    line-height: 2em;
/*  padding-top: 0.25em; */
}

.cellSelected {
    background: #00CCFF;
}

.cellUnselected {
    background: none;
}

.cellCoordinates {
    width: 36px;
    height: 36px;
    position: absolute;
    background: none;
    z-index: 1;
}
.autocorrMatrixContainer {
    position: absolute;
/*    float: left; */
/*    bottom: 0px; */
    display: block;
    background: none;
    font-family: Arial, Helvetica, Verdana, sans-serif;
}

.autocorrMatrixCell {
    width: 16px;
    height: 16px;
    position: absolute;
    z-index: 0;
    font-size: xx-small;
    font-weight: normal;
    font-style: normal;
    color: #FFFFFF;
    text-align: center;
    display: table-cell;
    vertical-align: middle;
    line-height: 2em;
/*  padding-top: 0.25em; */
}

.autocorrMatrixCellWrapper {
    width: 16px;
    height: 16px;
    position: absolute;
    background: none;
    z-index: 2;
    border-style: solid outset;
    border-width: 1px;
    border-color: black;
    font-size: x-small;
}

.autocorrMatrixCellCoordText {
    font-size: xx-small;
    font-weight: normal;
    font-style: normal;
    padding-top: 2px;
    padding-left: 3px;
    color: #444444;
    text-align: left;
    vertical-align: top;
}
.cellWrapper {
    width: 36px;
    height: 36px;
    position: absolute;
    background: none;
    z-index: 2;
    border-style: solid outset;
    border-width: 1px;
    border-color: black;
    font-size: normal;
}

.cellCoordText {
    font-size: x-small;
    font-weight: normal;
    font-style: normal;
    padding-top: 2px;
    padding-left: 3px;
    color: #444444;
    text-align: left;
    vertical-align: top;
}
\$\endgroup\$
  • \$\begingroup\$ if you used XHTML you could create a single element and give it specific attributes for each of the three things, and then accompany all the differences with different CSS classes using Javascript. \$\endgroup\$ – Malachi May 2 '14 at 15:52
  • 2
    \$\begingroup\$ @Malachi you can do the same in HTML5, using data attributes. \$\endgroup\$ – konijn May 2 '14 at 18:53
  • \$\begingroup\$ Please add your JS to this question, it is review worthy! \$\endgroup\$ – konijn May 2 '14 at 19:34
  • \$\begingroup\$ Yes I was adding it ;) I added everything to reproduce.. \$\endgroup\$ – Victor Gonzalez May 2 '14 at 19:36
5
\$\begingroup\$

Most interesting, a few pointers with 64 by 64 in mind.

  • .appendTo("#"+GRASP.config.gridContainer) <- That has got to be really slow. You access a property of an object/property of an object, concatenate and then use jQuery to do document.getElementById. Cache that jQuery result and go for that.

  • Except, every time you add an element, the browser rejiggles everything, which takes a surprising amount of calculations.. I would create a new div from scratch , attach all the elements under it and then add it to the document. This should speed up things tremendously.

  • I love how you actually already have that jQuery result cached ;)
    GRID_ELEMENT = $("#"+GRASP.config.gridContainer);

Not related to speed, but

  • I would rather go for $MATRIXHEADER then MATRIXHEADER_ELEMENT

  • There is a bit of copy pastage going on between createAutocorrelationMatrix and createGrid (the createElement part), you should resolve that into a common helper function

  • This:

    function cellClick(){
        var cell = $(this).next();
    
        if(cell.text() == "0"){
            cell.text("1");
        } else {
            cell.text("0");
        }
    }
    

    could be

    function cellClick(){
        var cell = $(this).next();
        cell.text( cell.text() == '0' ? '1' : '0');
    }
    

All in all, very readable code, I would not mind maintaining this.

\$\endgroup\$
  • \$\begingroup\$ Yeah for bullet 3 I was like, didn't I stored that in a variable ?? lol \$\endgroup\$ – Victor Gonzalez May 2 '14 at 20:09
1
\$\begingroup\$

I came up with quite a few changes, that I think may improve your speed quite dramatically.

Do note that I removed everything related to your Auto Correlation Matrix. I was not sure what it was for, and I wanted to keep my example short and clear. The changes I made for the grid should be easily applicable to that matrix as well however.

  • I don't understand why you need the 3 divs per cell. One div should be just fine! That is already 66% less DOM elements, and that should help...

    • In my version the coordinates are stored in a data attribute, and shown trough a css :before pseudo element on :hover. That eliminates 2 event listeners per cell and means you can use the much better css hover.
    • The value (0,1) is now stored in that one .cell div
    • The click handler is attached to that same div
    • Other advantages are that you don't need to use z-indexing and require a lot less css to display things correctly (that needs to be rendered for each cell)
  • The suggestion of mr. Rabbit to insert all your html at once, in stead of cell by cell, was a very good one, as it drastically reduces the number of reflows. So I went ahead and implemented that one.

  • I no longer position the cells absolute. You set the width and height of the wrapper, so just making them float left makes them automatically move into their correct position (just make sure to take the border width into account). This eliminates the need for all the position calculations that must take quite some time. If you want to keep the positions absolute, you should figure out some way of caching your coordinates. The y position is the same for all cells in row 1, the x postion is the same for all cells in column 1. Store the coordinates in some array map, and store them there the first time you calculate them and reuse them afterwards, or something in that order.
  • I tried to put as little code as possible inside the inner for loop, as it gets executed a lot of times. I create the cell in a single step and don't use the coord variable anymore as it would only get used once.
  • a little sidenote, why do you give your variables names in all caps? Isn't that only meant for constants? I kept them, as not relevant to the question, but find it very confusing...

Enough said, time for some code:

(function (GRASP, $) {
    var GRID_ROWS,
    GRID_COLS,
    GRID_ELEMENT;

    GRASP.config = {
        gridContainer: "grid",
        matrixContainer: "matrix",
        matrixHeader: "matrixHeader"
    };

    GRASP.start = function () {
        GRID_ROWS = $("#rows").val();
        GRID_COLS = $("#cols").val();
        createGrid();
    };

    function createGrid() {
        GRID_ELEMENT = $("#" + GRASP.config.gridContainer);
        var cell; // Contains the 1 or 0 based upon the cell selection
        var newGrid = $('<div id="grid" class="gridContainer" ></div>');


        for (var i = 1; i <= GRID_ROWS; i++) {
            for (var j = 1; j <= GRID_COLS; j++) {
                $("<div class='cell' data-hover-text='"+i+","+j+"'>0</div>")
                    .appendTo(newGrid)
                    .on("click", cellClick);
            }
        }

        newGrid.height(38 * GRID_ROWS);
        newGrid.width(38 * GRID_COLS);

        GRID_ELEMENT.replaceWith(newGrid);
    }

    function cellClick() {
        $(this).text($(this).text() == "0" ? "1" : "0");
    }


}(window.GRASP = window.GRASP || {}, jQuery));

$(document).ready(function () {
    GRASP.start();
});

And the updated fiddle (that contains the css for the coordinates on hover): http://jsfiddle.net/Tfs2M/2/

I did not do any benchmarks, but I am quite confident that this version is faster and more efficient. Anyway, I hope I gave you some good ideas and pointed in the right direction. Happy coding!

\$\endgroup\$
1
\$\begingroup\$

In addition to the other suggestions, this is a perfect use case for delegation. Rather than using on on each element (so remove this line):

.on("click", cellClick)

Use on on newGrid, giving it a selector:

newGrid.on('click', '.cell', cellClick);

This only needs to be done once, and reduces however many event listeners you'd have had before to just one.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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