[GitHub] [superset] villebro commented on a change in pull request #17758: feat(timeseries-chart): [WIP] add percentage threshold input control

[GitBox <gi...@apache.org>]

villebro commented on a change in pull request #17758:
URL: https://github.com/apache/superset/pull/17758#discussion_r772151643



##########
File path: superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts
##########
@@ -140,6 +142,7 @@ export default function transformProps(
       return prev + (value as number);
     }, 0);
     totalStackedValues.push(values);
+    thresholdValues.push((percentageThreshold / 100) * values);

Review comment:
       Since the controls sometimes handle missing/unset values in funny ways, I'd probably add this, just in case:
   ```js
       thresholdValues.push(((percentageThreshold || 0) / 100) * values);
   ```

##########
File path: superset-frontend/plugins/plugin-chart-echarts/test/Timeseries/transformProps.test.ts
##########
@@ -49,8 +53,8 @@ describe('EchartsTimeseries tranformProps', () => {
   };
 
   it('should tranform chart props for viz', () => {
-    const chartProps = new ChartProps(chartPropsConfig);
-    expect(transformProps(chartProps)).toEqual(
+    const chartProps: unknown = new ChartProps(chartPropsConfig);

Review comment:
       This specific line should be something like
   ```typescript
       const chartProps = new ChartProps<EchartsTimeseriesChartProps>(chartPropsConfig);
   ```
   However, it seems these types are slightly tangled up right now, and should probably be addressed in a follow-up PR, as I believe it's going to require some more type fixing. I'd also want to enable proper type checking for the test folders, but that also requires a separate 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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org