Feature/dc d3 dynamic charting #250
Conversation
Conflicts: .meteor/packages
|
Hey @apinf/developers , please review this PR. #212 - Related issue. |
|
|
||
| var stamp = moment(d._source.request_at); | ||
| stamp = stamp.format("YYYY-MM-DD-HH"); | ||
| d.ymd = dateFormat.parse(stamp); |
There was a problem hiding this comment.
This should be changed to some more general label such as timeStamp. timeStamp would correspond with timeStampDimension (line 43) and timeStampGroup (line 46).
There was a problem hiding this comment.
@brylie Time format changed to iso.
| }; | ||
|
|
||
| Template.chartLayout.events({ | ||
| "submit .filtering" : function(e){ |
There was a problem hiding this comment.
Consider using change instead of submit, as that would simplify usability. Also, consider identifying the "filtering" form more explicitly and by ID rather than class, since it will be the only filtering form on the page.
| <option value="12">December</option> | ||
| </select> | ||
| Year: | ||
| <select name="year"> |
There was a problem hiding this comment.
Consider how this year list could be programatically generated based on available data. Add a follow-up task for a future sprint.
There was a problem hiding this comment.
Open a task in the backlog to generate the list of months based on available data for a given year.
| </span> | ||
|
|
||
| </form> | ||
| <div class="sampleChart" id="chart"> |
There was a problem hiding this comment.
Follow the class name convention from other classes using dashes rather than camel case. Basically, the whole projecct, and certainly a single template, should use a common convention for class names, etc.
|
@frenchbread check out how to use the change event on your form:
|
|
@apinf/developers This one is done, so please review this PR and merge |
| @@ -1,49 +1,120 @@ | |||
| Template.chartLayout.rendered = function () { | |||
| Template.lineChart.rendered = function () { | |||
There was a problem hiding this comment.
Where is the lineChart.html? Place this rendered function within a corresponding lineChart.js in the same folder. E.g.
/client/views/dashboard/charts/line/lineChart.html
/client/views/dashboard/charts/line/lineChart.js
|
Fixed |
| @@ -0,0 +1,120 @@ | |||
| Template.lineChart.rendered = function () { | |||
There was a problem hiding this comment.
Following your convention, this should go in:
/client/charts/line/line.js
|
This is getting really close, but still has a few organizational issues. |
|
How about now? |
No description provided.