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


##########
superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts:
##########
@@ -1258,25 +1276,37 @@ export default function transformProps(
       // have less overlap, so disabling hideOverlap is safe.
       // At 0° rotation, also disable hideOverlap when showMaxLabel
       // is active so the forced boundary label is never suppressed
-      // by ECharts' overlap detection (#39899).
-      hideOverlap: showMaxLabel
-        ? false
-        : !(xAxisType === AxisType.Time && xAxisLabelRotation !== 0),
+      // by ECharts' overlap detection (#39899). Pinned ticks label
+      // every bucket, which does crowd, so thinning always wins there.
+      hideOverlap:
+        !!temporalTickValues ||
+        (showMaxLabel
+          ? false
+          : !(xAxisType === AxisType.Time && xAxisLabelRotation !== 0)),
       formatter: deduplicatedFormatter,
       rotate: xAxisLabelRotation,
       interval: xAxisLabelInterval,
       // Force the boundary labels on non-rotated time axes so the first
       // and last dates stay visible: hideOverlap can hide the last label,
       // and a min date that falls between "nice" ticks otherwise renders
       // no beginning label. Skipped when rotated to avoid phantom labels
-      // at the axis boundary.
-      ...(showMaxLabel && {
-        showMaxLabel: true,
-        alignMaxLabel: 'right',
-        showMinLabel: true,
-        alignMinLabel: 'left',
-      }),
+      // at the axis boundary. Also skipped for pinned ticks: the boundary
+      // buckets are already real ticks there, and showMaxLabel only shields
+      // its immediate neighbour — hideOverlap can still drop it against a
+      // farther label on a crowded weekly axis, reopening #39899.
+      ...(showMaxLabel &&
+        !temporalTickValues && {
+          showMaxLabel: true,
+          alignMaxLabel: 'right',
+          showMinLabel: true,
+          alignMinLabel: 'left',
+        }),
+      ...(temporalTickValues && { customValues: temporalTickValues }),
     },
+    // Gridlines, when shown, follow axisTick.customValues too.
+    ...(temporalTickValues && {
+      axisTick: { customValues: temporalTickValues },

Review Comment:
   `axisTick.customValues` renders one mark per bucket unconditionally while 
`hideOverlap` only thins `axisLabel`, so a 5-year weekly range draws 261 tick 
marks where echarts drew ~6 — cap it, or is the comb acceptable? Same at 
`MixedTimeseries/transformProps.ts:815`.



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