add dynamic chart support and demo #13
Conversation
- Reset clears the chart of all dataset elements. - Reload loads the data back into the chart again, recreating the dataset elements.
- The user can now alternate between multiple loaded dataset configurations on the same chart via a drop down selection.
tests/src/App.js
Outdated
| import dynamic_data_1 from './data/ts1.csv'; | ||
| import dynamic_data_2 from './data/ts2.csv'; | ||
|
|
||
| const d3 = require("d3"); |
There was a problem hiding this comment.
We need to include d3 in the package.json file defined for the test environment via yarn add d3
There was a problem hiding this comment.
Ok. I was wondering if you thought this was the best way to handle this or if should be done in a more react centric way as opposed to my d3 based approach. Thoughts?
There was a problem hiding this comment.
Since this is being referenced in the React environment, we should approach it from React's conventions and most of the ES6 conventions that come with the environment. For example, using ES6 style import statements: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/import
There was a problem hiding this comment.
Hmm, sorry I think I'm confused a bit. I emulated the const call as you wrote it in index.js -- what is the difference between import (as in App.js) and const (as in index.js)?
I probably should have been more explicit with my previous question, I was referring to my usage of d3 in App.js (in the setup_dynamic_chart_selection_box function).
There was a problem hiding this comment.
Both methods work for importing modules; however, are written differently depending on the style of JavaScript used. Since our testing environment uses webpack and Babel for transpiling newer JavaScript syntax to "browser-compatible" JavaScript, we can load modules using ES6 style imports.
import * as d3 from 'd3'; //ES6 style import
const d3 = require("d3"); //Vanilla JS style import
These conventions for imports have a few benefits such as referencing multiple named exports from a module or creating aliases for named exports.
|
Should we consider enabling console logging via a global library option? One may not want the extraneous logging in a production environment. |
|
I'm not opposed to updating the logging functionality, but I do think that should be part of a different PR. |
|
I agree! Can we also bump the version number with this PR? Since this is a considerable update, I'd like to publish these changes to the npm registry and use this in pbench dashboard: Line 3 in ebc20f8 |
|
I'm fine with bumping the version. The line you reference is in the tests directory, shouldn't the module version be somewhere else? Such as: Line 3 in ebc20f8 |
|
Yes! We should update the tests and module directory since this PR includes updates for both environments. |
2 participants
No description provided.