rusackas commented on code in PR #43180:
URL: https://github.com/apache/superset/pull/43180#discussion_r3952321031
##########
superset-frontend/packages/superset-ui-core/src/color/CategoricalColorScale.ts:
##########
@@ -207,6 +217,36 @@ class CategoricalColorScale extends ExtensibleFunction {
return color;
}
+ /**
+ * Colors that already belong to a label other than the given one, either
+ * because a custom label color forces them or because the shared dashboard
+ * color map assigned them. Such colors have an owner and must not be handed
+ * to a label that has no color yet.
Review Comment:
Good catch, fixed! Reworded to "avoided, when possible" to match
`getNextAvailableColor`'s own docstring and the actual fallback behavior.
##########
superset-frontend/packages/superset-ui-core/src/color/CategoricalColorScale.ts:
##########
@@ -154,9 +154,15 @@ class CategoricalColorScale extends ExtensibleFunction {
this.incrementColorRange();
}
- if (this.isColorUsed(color)) {
+ // colors owned by other labels are reserved: handing one of them to a
+ // brand new label would render two identical colors in this chart as
+ // soon as the owner label is resolved from the shared map or from a
+ // forced color, which happens after this call
+ const reservedColors = this.getReservedColors(cleanedValue);
+
+ if (this.isColorUsed(color) || reservedColors.has(color)) {
// fallback to least used color
- color = this.getNextAvailableColor(cleanedValue, color);
+ color = this.getNextAvailableColor(cleanedValue, color,
reservedColors);
Review Comment:
That path is the same on master, this PR doesn't touch how `currentColorMap`
picks the shared map over `chartLabelsColorMap` when there's no sliceId. Could
be a real edge case, but it's pre-existing and out of scope here. Open a
separate issue if you can reproduce it in practice.
--
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]
