sadpandajoe commented on code in PR #36214:
URL: https://github.com/apache/superset/pull/36214#discussion_r4364068951


##########
superset-frontend/plugins/plugin-chart-echarts/test/Timeseries/transformProps.test.ts:
##########
@@ -4637,3 +4637,253 @@ test('tooltip keeps the metric format on a Difference 
time comparison', () => {
   expect(result).toContain('$ 100');
   expect(result).toContain('$ 25');
 });
+
+describe('natural sort of a category x-axis (#35853)', () => {
+  function categoryChartProps(
+    records: { dim: string; metric: number }[],
+    formDataOverrides: Partial<EchartsTimeseriesFormData> = {},
+  ) {
+    return createTestChartProps({
+      formData: {
+        metrics: ['metric'],
+        groupby: [],
+        x_axis: 'dim',
+        ...formDataOverrides,
+      },
+      queriesData: [
+        createTestQueryData(records, {
+          colnames: ['dim', 'metric'],
+          coltypes: [GenericDataType.String, GenericDataType.Numeric],
+        }),
+      ],
+    });
+  }
+
+  test('reorders numeric-like string dimension values numerically, not 
lexicographically', () => {

Review Comment:
   Every numeric-like fixture in this block is a positive integer, so they all 
go through the BigInt branch; the `numeric` mode (decimals, e.g. `Number(sa) - 
Number(sb)`) is never exercised. If that branch regressed to a plain 
`localeCompare`, `['10.25', '2', '1.5']` would come out as `['1.5', '10.25', 
'2']` and every test here would still pass. Could you add a case with those 
values asserting `['1.5', '2', '10.25']` (plus a negative-integer one like 
`['-1', '-2', '-10']` -> `['-10', '-2', '-1']`)?



-- 
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]

Reply via email to