tclh123 / commits-graph

Git commits graph widget using HTML5/Canvas.
http://oct.tclh123.com/commits-graph/
226 stars 30 forks source link

Revised defaults for canvas size #2

Closed pseudocubic closed 10 years ago

pseudocubic commented 10 years ago

I was having a problem guessing at good widths and heights for canvases that would properly include all the data, but not be too big (I wanted the canvas to fit the data so that it would be easier to center on the page), so I added a method that determines the size of the canvas based on the data and either the provided or default step sizes.

In order to determine the max number of branches I needed a maximum finder function for nested lists; I used one from the d3.js javascript library, which is open source. I only took the function I needed, and included the license in the source, as required.

This only changes the default width and height, and still takes into account ones that are provided in options by the user. Therefore, if the user supplies no width and/or height, the canvas will fit the data in the dimension omitted by the user.

pseudocubic commented 10 years ago

Ah, so I made a silly assumption. Essentially, I assumed that the value of branch would never be higher than the maximum from or to, which would be incorrect if, for example, a user wanted to choose a different line color with a much higher value, in the case where they are designing a custom graph. In that case I figured that reusing an open source function to find the maximum over the whole set of nested arrays would be easiest, but obviously there are edge cases where this won't work properly.

To remedy this it is clear that finding the maximum of either from or to as you suggest is what I need to do, so I rewrote the function to find the number of branches in a similar vein as your suggestion:

var maxBranch = -1;
for (var i = 0; i < data.length; i++) { 
    for (var j = 0; j < data[i][2].length; j++) { 
        if (maxBranch < data[i][2][j][1] || maxBranch < data[i][2][j][2]) { 
            maxBranch = Math.max.apply(Math, [data[i][2][j][2], data[i][2][j][2]]); 
        } 
    } 
}

I also added some extra space to prevent parts of the graph from getting cut-off at the edges which I forgot originally.

tclh123 commented 10 years ago

LGTM ~