zhaoyongjie commented on code in PR #21449: URL: https://github.com/apache/superset/pull/21449#discussion_r969714188
########## superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts: ########## @@ -149,8 +148,9 @@ export default function transformProps( const colorScale = CategoricalColorNamespace.getScale(colorScheme as string); const rebasedData = rebaseForecastDatum(data, verboseMap); + // todo: if the both granularity_sqla and x_axis are `null`, should throw an error const xAxisCol = - verboseMap[xAxisOrig] || getColumnLabel(xAxisOrig || DTTM_ALIAS); + verboseMap[xAxisOrig] || getAxis(chartProps.rawFormData) || DTTM_ALIAS; Review Comment: Yes, the `DTTM_ALIAS` is a redundant case. I consider that if a chart can't have an axis here, the chart should throw an error, but there wasn't an appropriate approach to throw an error in the Chart and TS throw a type error, so I added a redundant condition here. I think we need to catch this error in the separated PR. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
