Release 0.3

Open source development across GitHub and other platforms tends to be a cyclical process. One takes a bug or feature request, debugs and trys to implement what has been outlined, and then creates a pull request with the fix. Of course, as a developer, you can find your own bugs and fix those or post them as an issue for others to view and fix but I feel overall you end up doing the same thing which is not exactly a bad. Open source provides an existential amount of real-world experience working on huge industry leading projects some of which I have added to my own personal resume. However, after being so involved in the open source community and putting out three or four substantial pull requests for each release, I decided to take it a bit easier on constantly fixing bugs. It took a lot of my time to fix some of the bugs I went after and was mentally draining to be trying to fix or create something constantly. I don’t regret spending most of my time fixing bugs however for this release it was hard to jump in with the same energy as I did previously.

Does this mean future releases will be small? No. This release is small purely out of choice, mental break, and the fact that I’ve been developing other private repos on GitHub for another project for school. With two months left in my college career, I’ve also been focusing on relocating for work and learning new skills for my job upon graduation which has taken up a lot of my time as well (it’s hard to code and travel to look at apartments on the weekend). Enough of the excuses and reasons lets jump into where I’m at with  Release 0.3!

For this release, I decided to take on an interesting bug in Chart.js. I found this bug to me almost one of the most interesting ones I’ve seen.

Example: 

chart1.gif

The main problem here is that the tooltip, which is the black box containing the data is going off the screen and following the data as if it is continuing. It’s weird because in this example it’s only supposed to print the last four hundred days however its actually printing more than the minimum value than where the data is initially supposed to start at. After analyzing the issue I initially thought it was a data problem since its not sticking to the minimum value assigned but one of the maintainers steered me away from investigating further into that idea.

chart2

I began investigating the file in the project that contains all the tooltip code called core.tooltip.js. Where I added the following logic.

var border = active[0]._xScale.left + 7; // 7 used for graph offset
if (existingModel.x > border) {
    for (i = 0, len = active.length; i < len; ++i) {
        tooltipItems.push(createTooltipItem(active[i]));
    }
}

The problem with this logic is that it is not secure and breaks several tests associated with the project. Chart.Js can produce a variety of different charts, for example line graphs, pie charts, and bubble charts. The implementation I originally provided only works for a line graph and doesn’t account for the difference of attributes in different graph types. The result of having this implementation was several errors in the build test for different charts.

After revaluating core.tooltip.js I found the code that deals with the mouse event and tried to implement what I had previously did in the update() function. I ended up having to make a global variable that retrieved the chart area so I could use it to find the boundaries of the chart. For example:

var globalChartArea;
...
globalChartArea = chartArea.left;
...
if (e.type === 'mouseout') {
    me._active = [];
}
else if(e.x > globalChartArea) {
    me._active = me._chart.getElementsAtEventForMode(e, options.mode, options);
}

Developing this block of code was mainly the product of not knowing what variables can access certain objects which was later clarified by one of the lead developers of the project.

chart3

With this and more feedback from the Chart.js developers I created what I thought would be my final commit to this PR. Result: 

var border = me._chart.chartArea;
me._lastActive = me._lastActive || [];
if (e.type === 'mouseout') {
   me._active = [];
} else if (e.x > border.left && e.x < border.right) {
   me._active = me._chart.getElementsAtEventForMode(e, options.mode, options);
}

To my surprise there was small bug in the code outlined by developer Simon Brunel that I hadn’t noticed before.

chart4.gif

chart5.JPG

Simon also noted that we need updated unit tests for this fix which was an added task I had to accomplish. Taking Simon’s other code suggestions I sent a commit which would end up being the final fix for this issue. Result: 

var area = me._chart.chartArea;
if (e.type === 'mouseout' || e.x < area.left || e.x > area.right || e.y < area.top || e.y > area.bottom) {
     me._active = [];
} else {
    me._active = me._chart.getElementsAtEventForMode(e, options.mode, options);
}

This code fixed the issue Simon outlined leaving only the unit tests that had to be updated. The main problem with the unit test for this fix is that currently it only accounted for the mouse position of x and not y. To fix this I added the implementation used to get the x coordinates and modified it so it would work to collect the y coordination of the mouse. This seemed to fix the problem with the unit tests and got approved by one of the project developers Etimberg.

chart6.JPG

Now it is mainly waiting for a merge or request of changes on the fix. Overall I actually had a lot fun with this bug, it is something I was really interested in fixing and enjoyed the correspondence and collaboration with the other Chart.js developers. I plan for my next release to be a fair bit larger however the fix for this bug wasn’t as simple as I thought and was a great learning experience.

 

Advertisements

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out /  Change )

Google+ photo

You are commenting using your Google+ account. Log Out /  Change )

Twitter picture

You are commenting using your Twitter account. Log Out /  Change )

Facebook photo

You are commenting using your Facebook account. Log Out /  Change )

w

Connecting to %s