As shown above, the main problem here is that the initial label in the array is centered however each consecutive label in the array is being printed below it and not being centered. To fix this initially I wrote:
context.translate(0, -tickFont.size * (label.length / 2));
which moves the labels up by the font size multiplied by the length of the array divided by two. This equation essentially finds the spacing between one label and the other and moves it up by that amount of space. For example to move “Row two” to the position of “Row one” it would need to be translated by negative eighteen. This was what I put up in my first pull request and as a result I got some good feedback.
It turns out my initial approach to fixing this issue had some flaws and thankfully I got some great guidance instead of having my pull request closed instantly like when I was working on other projects. Pettiness aside the problem with the code was that it was slower to translate the label via the context than to move them using “y”. “y” in the code sets the initial printing position of the label so offsetting “y” would create an easier solution than translating the labels after the fact which is something I hadn’t thought of. Also if the graph was in vertical mode my code wouldn’t account for it and it ended up moving the labels into the bars.
Taking all the feedback into consideration I began writing code to update the pull request with the necessary changes.
This code also had some minor problems like using ternary operator instead of an if statement to check if the graph is horizontal and using predefined variables in the for loop. I began working with Simon Brunel who is one of the lead developers for the project so we could both find the right equation. It was apparent to me that we both different ideas on how to write the equation and some which were both incorrect.
In the pictures above Simons comment is the result of my equation and my comment is the result when using his equation. We were both slightly off but I’m glad I had someone to help me with the process. From this point, it seemed like we were both awfully close and were missing the small thing that would make the equation work for all cases. Shortly after that comment, I decided to try to divide the whole equation by 1.2 which was the hardcoded line space value in the project. I thought this worked, however, Simon found the real reason the equation wasn’t working.
Simons new equation ended up working thankfully and I put up an updated PR that got approved and merged shortly after. Although it wasn’t me who figured out the correct equation, in the end, I still felt like a fundamental part of the development process. Also being new to the project I didn’t know the key terms and variables that would influence the creation of the correct equation. Overall I really enjoyed working on this project and hope to tackle other bugs in the future I loved all the feedback and help which will keep me coming back for more bugs!
Pull requests that were wanted features but were rejected for this release: