codeant-ai-for-open-source[bot] commented on code in PR #37531:
URL: https://github.com/apache/superset/pull/37531#discussion_r2738503195
##########
superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts:
##########
@@ -373,6 +375,53 @@ export default function transformProps(
}
});
+ // Add x-axis color legend when showColorByXAxis is enabled
+ if (showColorByXAxis && groupBy.length === 0 && series.length > 0) {
+ // Hide original series from legend
+ series.forEach(s => {
+ s.legendHoverLink = false;
+ });
+
+ // Get x-axis values from the first series
+ const firstSeries = series[0];
+ if (firstSeries && Array.isArray(firstSeries.data)) {
+ const xAxisValues: (string | number)[] = [];
+
+ // Extract x-axis values
+ (firstSeries.data as any[]).forEach(point => {
+ let xValue;
+ if (point && typeof point === 'object' && 'value' in point) {
+ const val = point.value;
+ xValue = Array.isArray(val) ? val[0] : val;
+ } else if (Array.isArray(point)) {
+ xValue = point[0];
+ } else {
+ xValue = point;
+ }
+ xAxisValues.push(xValue);
+ });
+
+ // Create hidden series for legend (using 'line' type to not affect bar
width)
+ xAxisValues.forEach(xValue => {
+ const colorKey = String(xValue);
+ series.push({
+ name: String(xValue),
Review Comment:
**Suggestion:** Performance & correctness: when creating hidden legend
series you append one series per x-axis data point without deduplicating, which
can create duplicate legend entries and a large number of unnecessary series;
deduplicate x-axis values first before pushing hidden series. [performance]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Timeseries chart legend shows duplicate entries.
- ⚠️ Increased series[] size slows ECharts rendering.
- ⚠️ Legend clutter harms user color interpretation.
```
</details>
```suggestion
// Deduplicate x-axis values to avoid duplicate legend entries and
excessive series
const uniqueXValues = Array.from(
new Set(
xAxisValues
.map(v => (v === null || v === undefined ? String(v) :
String(v))),
),
);
uniqueXValues.forEach(xValue => {
const colorKey = String(xValue);
series.push({
name: xValue,
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Open the Timeseries chart code path: transformProps in
plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts (function
transformProps).
The x-axis legend augmentation code lives at lines ~378-423 and the
hidden-series creation
is at lines 404-421.
2. Enable the new option in formData: showColorByXAxis = true and ensure
groupBy is empty
(groupBy.length === 0). These formData values are read at the top of
transformProps
(extracted into showColorByXAxis and groupBy).
3. Provide a dataset such that the first rendered series (series[0])
contains repeated x
values (e.g., firstSeries.data contains multiple points with the same x
value).
transformProps extracts xAxisValues by iterating firstSeries.data and
pushing every x it
finds (lines 390-402).
4. transformProps then executes the hidden-series creation loop at lines
404-421, pushing
one hidden series per xAxisValues entry (no deduplication). Observe
duplicate legend items
in the rendered chart legend (legend.data is later driven by legendData
built from the
same firstSeries.data at lines ~565-582), and a larger-than-necessary
series[] length
causing extra work when ECharts processes series.
```
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:**
superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts
**Line:** 405:408
**Comment:**
*Performance: Performance & correctness: when creating hidden legend
series you append one series per x-axis data point without deduplicating, which
can create duplicate legend entries and a large number of unnecessary series;
deduplicate x-axis values first before pushing hidden series.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
```
</details>
##########
superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts:
##########
@@ -513,14 +562,34 @@ export default function transformProps(
isHorizontal,
);
- const legendData = rawSeries
- .filter(
- entry =>
- extractForecastSeriesContext(entry.name || '').type ===
- ForecastSeriesEnum.Observation,
- )
- .map(entry => entry.name || '')
- .concat(extractAnnotationLabels(annotationLayers));
+ const legendData = showColorByXAxis && groupBy.length === 0 && series.length
> 0
+ ? // When showColorByXAxis is enabled, show only x-axis values
+ (() => {
+ const firstSeries = series[0];
+ if (firstSeries && Array.isArray(firstSeries.data)) {
+ return (firstSeries.data as any[]).map(point => {
+ if (point && typeof point === 'object' && 'value' in point) {
+ const val = point.value;
+ return String(Array.isArray(val) ? val[0] : val);
+ } else if (Array.isArray(point)) {
+ return String(point[0]);
+ } else {
+ return String(point);
+ }
+ });
+ }
Review Comment:
**Suggestion:** Logic/robustness: `legendData` is built from
`firstSeries.data` and can include duplicate or empty/undefined entries; filter
out empty/undefined names and deduplicate them so legend items are stable and
meaningful. [logic error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Timeseries legend shows empty or duplicated labels.
- ⚠️ Legend sorting and selection behave inconsistently.
- ⚠️ User confusion interpreting color-by-x legend.
```
</details>
```suggestion
? // When showColorByXAxis is enabled, show only x-axis values (filtered
+ deduped)
(() => {
const firstSeries = series[0];
if (firstSeries && Array.isArray(firstSeries.data)) {
const names = (firstSeries.data as any[]).map(point => {
if (point && typeof point === 'object' && 'value' in point) {
const val = point.value;
return String(Array.isArray(val) ? val[0] : val);
} else if (Array.isArray(point)) {
return String(point[0]);
} else {
return String(point);
}
})
// filter out empty/invalid entries and deduplicate
.filter(name => name !== '' && name !== 'undefined' && name !==
'null');
return Array.from(new Set(names));
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Call transformProps in
plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts
with formData.showColorByXAxis = true and no groupBy (groupBy.length === 0).
The
legendData computation is at lines ~565-592.
2. If the first rendered series (series[0]) contains points where the x
value is
null/undefined/empty, the mapping at lines 569-578 will convert those to the
strings
"null" or "undefined" or empty strings, and include them in the returned
array.
3. If the firstSeries.data contains duplicate x values, the current map
returns duplicates
(no deduplication). These values are later fed to echartOptions.legend.data
(see legend
configuration at lines ~771-792), producing duplicate or meaningless legend
entries in the
chart UI.
4. Reproduce in UI: render a Timeseries chart with showColorByXAxis enabled
and a dataset
where firstSeries.data includes duplicate or empty x values; verify
duplicate/empty legend
items appear. Fixing by filtering out empty values and deduplicating (as
suggested) yields
stable, meaningful legend items.
```
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:**
superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts
**Line:** 566:580
**Comment:**
*Logic Error: Logic/robustness: `legendData` is built from
`firstSeries.data` and can include duplicate or empty/undefined entries; filter
out empty/undefined names and deduplicate them so legend items are stable and
meaningful.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
```
</details>
--
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]
