aminghadersohi commented on code in PR #42910:
URL: https://github.com/apache/superset/pull/42910#discussion_r4417891512
##########
superset-frontend/src/explore/components/controls/ColorPickerControl.tsx:
##########
@@ -49,13 +63,20 @@ export interface ColorPickerControlProps {
ariaLabel?: string;
resolveThemeTokens?: boolean;
outputFormat?: ColorOutputFormat;
+ disabled?: boolean;
}
const normalizeColorToHex = (color: string): string => {
if (!color) return '';
if (color.startsWith('#')) {
- return color.toLowerCase();
+ const lower = color.toLowerCase();
+ // Expand 3/4-digit shorthand so it compares equal to the six/eight-digit
+ // hex the picker emits.
+ if (/^#[0-9a-f]{3,4}$/.test(lower)) {
Review Comment:
This expansion also feeds `getReverseThemeColorMap`, so with the default
theme a pure black or white pick now saves `colorTextBase`/`colorBgBase`
(`#000`/`#fff`). Table conditional formatting then emits invalid `#00070`, and
the PoP pill text matches its background.
##########
superset-frontend/plugins/plugin-chart-echarts/src/Bullet/transformProps.ts:
##########
@@ -103,23 +130,57 @@ export default function transformProps(
theme.colorFillSecondary,
theme.colorFill,
];
+ // Custom colors (if any) are authored positionally against the original,
+ // pre-sort `ranges` order via the `range_colors` control -- capture each
+ // range's color here, before the descending sort below reorders them.
const sortedRanges = [...ranges]
- .map((value, i) => ({ value, label: rangeLabels[i] }))
+ .map((value, i) => ({
+ value,
+ label: rangeLabels[i],
+ // Colors only apply to user-entered ranges; the automatic fallback band
+ // must not pick up a stale `range_colors` entry kept from earlier edits.
+ color: explicitRanges ? rangeColors?.[i] || undefined : undefined,
+ }))
.filter(({ value }) => value > axisMin)
.sort((a, b) => b.value - a.value);
// Each band's label sits inside its own top-right corner: the visible strip
// of a nested band ends at its threshold, so the label lands inside the
// range it names and the rightmost label cannot clip at the chart edge.
- const bands = sortedRanges.map(({ value, label }, i) => ({
+ const getBandLabelColor = (fill?: string): string => {
+ const luminance = fill ? getColorLuminance(fill) : null;
+ if (luminance === null) return theme.colorTextSecondary;
+ // Fixed light/dark foregrounds, not `theme.colorText`: that token flips
+ // to a light color in dark themes and would vanish on a light custom
+ // fill. Pick whichever foreground actually contrasts better against
+ // this fill, rather than a single magic luminance cutoff that doesn't
+ // match where white and the dark foreground really break even.
+ return wcagContrast(luminance, BAND_LABEL_LUMINANCE.light) >=
+ wcagContrast(luminance, BAND_LABEL_LUMINANCE.dark)
+ ? BAND_LABEL_COLORS.light
+ : BAND_LABEL_COLORS.dark;
+ };
+ // Bands are drawn outermost first, so a band without its own color sits on
+ // top of the nearest outer custom fill; its label must contrast with that.
+ let outerCustomFill: string | undefined;
+ const bandLabelFills = sortedRanges.map(({ color }) => {
+ if (color) outerCustomFill = color;
+ return color ?? outerCustomFill;
+ });
Review Comment:
Nit: A custom fill with alpha below 0.5 still becomes its own label fill, so
the label ignores an opaque outer custom band showing through:
`ranges="50,100"`, `range_colors=['#ffffff33','#000000']` puts dark text on
near-black.
```suggestion
// Bands are drawn outermost first, so a band without its own opaque color
// shows the nearest outer opaque custom fill; its label must contrast with
// that.
let outerCustomFill: string | undefined;
const bandLabelFills = sortedRanges.map(({ color }) => {
if (color && getColorLuminance(color) !== null) outerCustomFill = color;
return outerCustomFill;
});
```
--
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]
