Closed Lucytheanimefan closed 8 years ago
@balhoff feel free to use this as an opportunity to provide code review comments.
@Lucytheanimefan looking nice! The most immediate piece of feedback I have is wondering whether you can draw the graphs within a particular DOM element. Right now they are being appended to the end of the page after the footer. The function where the drawing is implemented gets 3 arguments: link: function(scope, element, attrs)
. All the content for this directive should go inside of element
. Is that an easy fix?
It would be nice to add a scope variable where you could set true
or false
if the graph is busy retrieving data. The app could watch this variable and put up a busy indicator (you don't need to put this functionality in your directive).
@balhoff can you comment whether you consider this ready for merge now?
@Lucytheanimefan nice fixes! The graphs are placed where I expect now.
Enabled drilling down on graph, added css styling, and incorporated other more recent updates such as clickable x-axis labels