-
Notifications
You must be signed in to change notification settings - Fork 4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Dynamic configuration and visual props #29
Changes from all commits
764a682
4c69490
f1c3390
d52ee53
5a26a81
45f3717
37e1186
083c8c0
1c1e596
ad4655b
f9248c5
2f0bf60
0296683
b00eed5
65d1a8b
daef993
8590e2d
3a0ea84
a5c84f4
e7a234d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,6 +8,10 @@ | |
* Copyright: ThoughtSpot Inc. 2023 | ||
*/ | ||
|
||
import { | ||
ValidationResponse, | ||
VisualPropEditorDefinition, | ||
} from '@thoughtspot/ts-chart-sdk'; | ||
import { | ||
ChartColumn, | ||
ChartConfig, | ||
|
@@ -21,7 +25,9 @@ import { | |
Query, | ||
VisualProps, | ||
} from '@thoughtspot/ts-chart-sdk'; | ||
import { ChartConfigEditorDefinition } from '@thoughtspot/ts-chart-sdk/src'; | ||
import Chart from 'chart.js/auto'; | ||
import { toDimension } from 'chart.js/dist/helpers/helpers.core'; | ||
import ChartDataLabels from 'chartjs-plugin-datalabels'; | ||
import _ from 'lodash'; | ||
|
||
|
@@ -37,22 +43,9 @@ const visualPropKeyMap = { | |
2: 'accordion.datalabels', | ||
}; | ||
|
||
const numberFormatter = value => { | ||
if (value > 1000000000) { | ||
return (value / 1000000000).toFixed(2) + 'B'; | ||
} | ||
if (value > 1000000) { | ||
return (value / 1000000).toFixed(2) + 'M'; | ||
} | ||
if (value > 1000) { | ||
return (value / 1000).toFixed(2) + 'K'; | ||
} | ||
return value; | ||
}; | ||
|
||
function getDataForColumn(column: ChartColumn, dataArr: DataPointsArray) { | ||
const idx = _.findIndex(dataArr.columns, colId => column.id === colId); | ||
return _.map(dataArr.dataValue, row => row[idx]); | ||
const idx = _.findIndex(dataArr.columns, (colId) => column.id === colId); | ||
return _.map(dataArr.dataValue, (row) => row[idx]); | ||
} | ||
|
||
function getColumnDataModel( | ||
|
@@ -151,6 +144,11 @@ function render(ctx: CustomChartContext) { | |
visualPropKeyMap[2], | ||
false, | ||
); | ||
const labelColor = _.get( | ||
chartModel.visualProps, | ||
visualPropKeyMap[1], | ||
availableColor[0], | ||
); | ||
if (!dataModel) { | ||
return; | ||
} | ||
|
@@ -174,11 +172,8 @@ function render(ctx: CustomChartContext) { | |
plugins: { | ||
// Change options for ALL labels of THIS CHART | ||
datalabels: { | ||
display: allowLabels ? 'auto' : false, | ||
formatter: value => numberFormatter(value), | ||
color: 'blue', | ||
textStrokeColor: 'white', | ||
textStrokeWidth: 5, | ||
Comment on lines
-178
to
-181
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. how are these properties working today? |
||
display: allowLabels, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. display takes true now? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. https://chartjs-plugin-datalabels.netlify.app/guide/positioning.html#rotation will depend on implementation, true is also a valid value as mentioned above in the documentation |
||
color: labelColor, | ||
labels: { | ||
title: { | ||
font: { | ||
|
@@ -268,12 +263,12 @@ const renderChart = async (ctx: CustomChartContext): Promise<void> => { | |
|
||
const measureColumns = _.filter( | ||
cols, | ||
col => col.type === ColumnType.MEASURE, | ||
(col) => col.type === ColumnType.MEASURE, | ||
); | ||
|
||
const attributeColumns = _.filter( | ||
cols, | ||
col => col.type === ColumnType.ATTRIBUTE, | ||
(col) => col.type === ColumnType.ATTRIBUTE, | ||
); | ||
|
||
const axisConfig: ChartConfig = { | ||
|
@@ -311,35 +306,77 @@ const renderChart = async (ctx: CustomChartContext): Promise<void> => { | |
); | ||
return queries; | ||
}, | ||
renderChart: ctx => renderChart(ctx), | ||
chartConfigEditorDefinition: [ | ||
{ | ||
key: 'column', | ||
label: 'Custom Column', | ||
descriptionText: | ||
'X Axis can only have attributes, Y Axis can only have measures, Color can only have attributes. ' + | ||
'Should have just 1 column in Y axis with colors columns.', | ||
columnSections: [ | ||
{ | ||
key: 'x', | ||
label: 'Custom X Axis', | ||
allowAttributeColumns: true, | ||
allowMeasureColumns: false, | ||
allowTimeSeriesColumns: true, | ||
maxColumnCount: 1, | ||
}, | ||
{ | ||
key: 'y', | ||
label: 'Custom Y Axis', | ||
renderChart: (ctx) => renderChart(ctx), | ||
validateConfig: ( | ||
updatedConfig: any[], | ||
chartModel: any, | ||
): ValidationResponse => { | ||
if (updatedConfig.length <= 0) { | ||
return { | ||
isValid: false, | ||
validationErrorMessage: ['Invalid config. no config found'], | ||
}; | ||
} else { | ||
return { | ||
isValid: true, | ||
}; | ||
} | ||
}, | ||
chartConfigEditorDefinition: ( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. for backward compatibility verification should we keep this as it is? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have checked the backwards compatibilty it is working for both object and function we can keep it like this for a sample chart. |
||
currentChartConfig: ChartModel, | ||
ctx: CustomChartContext, | ||
): ChartConfigEditorDefinition[] => { | ||
const { config, visualProps } = currentChartConfig; | ||
|
||
const yColumns = config?.chartConfig?.[0]?.dimensions.find( | ||
(dimension) => dimension.key === 'y' && dimension.columns, | ||
); | ||
|
||
const configDefinition = [ | ||
{ | ||
key: 'column', | ||
label: 'Custom Column', | ||
descriptionText: | ||
'X Axis can only have attributes, Y Axis can only have measures, Color can only have attributes. ' + | ||
'Should have just 1 column in Y axis with colors columns.', | ||
columnSections: [ | ||
{ | ||
key: 'x', | ||
label: 'Custom X Axis', | ||
allowAttributeColumns: true, | ||
allowMeasureColumns: false, | ||
allowTimeSeriesColumns: true, | ||
maxColumnCount: 1, | ||
}, | ||
{ | ||
key: 'y', | ||
label: 'Custom Y Axis', | ||
allowAttributeColumns: false, | ||
allowMeasureColumns: true, | ||
allowTimeSeriesColumns: false, | ||
}, | ||
], | ||
}, | ||
]; | ||
if (yColumns?.columns.length) { | ||
for (let i = 0; i < yColumns.columns.length; i++) { | ||
configDefinition[0].columnSections.push({ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. where are we using this? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This I have added for an example purpose, when we are adding measures to y-axis, a new layer will be created with the same. |
||
key: `layers${i}`, | ||
label: `Measures layer${i}`, | ||
allowAttributeColumns: false, | ||
allowMeasureColumns: true, | ||
allowTimeSeriesColumns: false, | ||
}, | ||
], | ||
}, | ||
], | ||
visualPropEditorDefinition: { | ||
elements: [ | ||
}); | ||
} | ||
} | ||
return configDefinition; | ||
}, | ||
visualPropEditorDefinition: ( | ||
currentVisualProps: ChartModel, | ||
ctx: CustomChartContext, | ||
): VisualPropEditorDefinition => { | ||
const { visualProps } = currentVisualProps; | ||
const elements = [ | ||
{ | ||
key: 'color', | ||
type: 'radio', | ||
|
@@ -352,13 +389,6 @@ const renderChart = async (ctx: CustomChartContext): Promise<void> => { | |
key: 'accordion', | ||
label: 'Accordion', | ||
children: [ | ||
{ | ||
key: 'Color2', | ||
type: 'radio', | ||
defaultValue: 'blue', | ||
values: ['blue', 'white', 'red'], | ||
label: 'Color2', | ||
}, | ||
{ | ||
key: 'datalabels', | ||
type: 'toggle', | ||
|
@@ -367,7 +397,19 @@ const renderChart = async (ctx: CustomChartContext): Promise<void> => { | |
}, | ||
], | ||
}, | ||
], | ||
]; | ||
if (visualProps?.length !== 0) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is not an array but an object right? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. element is an array, visualPropEditordefinition is an Object |
||
if (visualProps?.accordion?.datalabels) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. whats the value in this change? I see no difference? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done |
||
elements[1].children?.push({ | ||
key: 'Color2', | ||
type: 'radio', | ||
defaultValue: 'blue', | ||
values: ['blue', 'white', 'red'], | ||
label: 'Color2', | ||
}); | ||
} | ||
} | ||
return { elements }; | ||
}, | ||
}); | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we seem to have removed a lot of properties... is this working as expected?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is working