fix(data-analysis): charts update functionality and extras update#3614
fix(data-analysis): charts update functionality and extras update#3614
Conversation
zdrawku
added
status: in-test
status: verified
and removed
status: awaiting-test
status: in-test
labels
| this.selectedCharts[c] = this.chartIntegration.chartFactory(c, null, this.selectedCharts[c]); | ||
| } else { | ||
| chartHost.viewContainerRef.clear(); | ||
| this.selectedCharts[c] = this.chartIntegration.chartFactory(c, chartHost.viewContainerRef); |
There was a problem hiding this comment.
Export the this.selectedCharts[c] into a constant. This is the third usage in few lines.
| } | ||
|
|
||
| public createChartCommonLogic() { | ||
| if (Object.keys(this.selectedCharts).length !== 0) { |
There was a problem hiding this comment.
This if is not needed.
Together with the settimeout on next line brings unnecessary complexity that is not even understandable why the code is like this.
Drop the if and document the setTimeout usage.
| this.chartIntegration.disableCharts(this.allCharts); | ||
| } else { | ||
| args.chartsAvailability.forEach((isAvailable, chart) => { | ||
| if (args.chartsForCreation.indexOf(chart) === -1) { |
There was a problem hiding this comment.
That's just style preference - maybe includes would be easier to read.
| } else { | ||
| args.chartsAvailability.forEach((isAvailable, chart) => { | ||
| if (args.chartsForCreation.indexOf(chart) === -1) { | ||
| this.chartIntegration.disableCharts([chart]); |
There was a problem hiding this comment.
Am not sure what is the implementation of disable/enableCharts function in chartIntegration package. Possibly it would be more effective and concise, if the method is called once for all charts to enable/disable.
| (click)="createChart(chart)" | ||
| title="{{chart}}" | ||
| [ngClass]="{'disabled': allCharts.indexOf(chart) === -1, 'selected': chart | hastDuplicateLayouts: dock.layout: selectedCharts}" | ||
| [ngClass]="{'disabled': availableCharts.indexOf(chart) === -1, 'selected': chart | hastDuplicateLayouts: dock.layout: selectedCharts}" |
There was a problem hiding this comment.
Possibly use includes, better readibility in the template
3 participants
Related to #3599