EnxDev commented on code in PR #43339:
URL: https://github.com/apache/superset/pull/43339#discussion_r3851744730


##########
superset-frontend/plugins/plugin-chart-echarts/src/utils/series.ts:
##########
@@ -986,6 +987,45 @@ export function getAxisType(
   return AxisType.Category;
 }
 
+/**
+ * Bucket timestamps a temporal axis should tick on, or undefined to let 
ECharts
+ * choose.
+ *
+ * ECharts generates time ticks from a calendar ladder with no week unit, so 
for
+ * weekly data it steps days from the 1st of each month instead: labels drift
+ * across weekdays and snap to month starts (#17226). Coarser grains already 
land
+ * on their data and keep ECharts' calendar-nice labels.
+ */
+export function getTemporalTickValues(
+  data: DataRecord[],
+  xAxisLabel: string,
+  xAxisType: AxisType,
+  timeGrain?: string,
+): number[] | undefined {
+  if (
+    xAxisType !== AxisType.Time ||
+    !timeGrain ||
+    !WEEKLY_TIME_GRAINS.has(timeGrain)
+  ) {
+    return undefined;
+  }
+  const values = new Set<number>();
+  data.forEach(row => {
+    const value = row[xAxisLabel];
+    const timestamp =
+      // eslint-disable-next-line no-nested-ternary
+      value instanceof Date
+        ? value.getTime()
+        : typeof value === 'string'
+          ? new Date(value).getTime()

Review Comment:
   Real bug, confirmed against ECharts source. Fixed with parseTemporalString()



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