bfs: support all charts #13
Conversation
| // Seems like the registered chart components change some | ||
| // props in-place, which leads to unwanted side effects. Therefore, | ||
| // we only pass a copy. | ||
| JSON.parse(JSON.stringify(feature)), |
There was a problem hiding this comment.
minor: maybe lodash.clone(..) could be used
| // Seems like the registered chart components change some | ||
| // props in-place, which leads to unwanted side effects. Therefore, | ||
| // we only pass a copy. | ||
| JSON.parse(JSON.stringify(chart.feature)), |
There was a problem hiding this comment.
minor: maybe lodash.clone(..) could be used
| recreateCharts = true; | ||
| } | ||
| if ( | ||
| JSON.stringify(currentChartConfigs) !== |
There was a problem hiding this comment.
minor: maybe lodash.isEqual(...) could be used
| @@ -108,27 +106,48 @@ export const groupByLocation = ( | |||
| const currentLocation = locations[loc]; | |||
|
|
|||
| if (Array.isArray(currentLocation)) { | |||
| currentLocation.push(d); | |||
| const item = { | |||
There was a problem hiding this comment.
minor: maybe use loadash function here: https://lodash.com/docs/4.17.15#clone
| const transformedProps = chartTransformer(config); | ||
|
|
||
| const feature: ChartConfigFeature = { | ||
| type: 'Feature', | ||
| geometry: JSON.parse(location), | ||
| properties: transformedProps, | ||
| properties: { |
There was a problem hiding this comment.
minor: maybe use lodash function here https://lodash.com/docs/4.17.15#clone
| const c = await registry.getAsPromise(vizType); | ||
| setChart(() => c); |
There was a problem hiding this comment.
writing the full variable name is better to understand IMHO
| const c = await registry.getAsPromise(vizType); | |
| setChart(() => c); | |
| const chart = await registry.getAsPromise(vizType); | |
| setChart(() => chart); |
| const chartLayer = layers | ||
| .getArray() | ||
| .filter(layer => layer instanceof ChartLayer)[0] as ChartLayer; |
There was a problem hiding this comment.
maybe find makes the code slightly easier to read https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/find
| const { r, g, b, a } = chartBackgroundColor; | ||
| const cssColor = `rgba(${r}, ${g}, ${b}, ${a})`; |
There was a problem hiding this comment.
move outside of if/else block for minor simplification
| const { r, g, b, a } = chartBackgroundColor; | ||
| const cssColor = `rgba(${r}, ${g}, ${b}, ${a})`; |
There was a problem hiding this comment.
move outside of if/else block for minor simplification
Welcome to Codecov 🎉Once merged to your default branch, Codecov will compare your coverage reports and display the results in this comment. Thanks for integrating Codecov - We've got you covered ☂️ |
This adds support for any chart type - not only echarts. This is done retrieving the required component from the component registry. Also some bugs were fixed.
@JakobMiksch please review