Re: [PR] feat: add custom colors and thresholds for CountryMap plugin [superset]
Facyla commented on PR #36103: URL: https://github.com/apache/superset/pull/36103#issuecomment-3669611901 Hi, The changes have been reintegrated for a cleaner integration based on rc4 master, and proposed in a new PR: https://github.com/apache/superset/pull/36732 -- 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]
Re: [PR] feat: add custom colors and thresholds for CountryMap plugin [superset]
Facyla closed pull request #36103: feat: add custom colors and thresholds for CountryMap plugin URL: https://github.com/apache/superset/pull/36103 -- 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]
Re: [PR] feat: add custom colors and thresholds for CountryMap plugin [superset]
Facyla commented on PR #36103: URL: https://github.com/apache/superset/pull/36103#issuecomment-3637408445 Hi, I've converted this PR to draft to handle master changes merge and produce a more readable PR. -- 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]
Re: [PR] feat: add custom colors and thresholds for CountryMap plugin [superset]
codeant-ai-for-open-source[bot] commented on code in PR #36103:
URL: https://github.com/apache/superset/pull/36103#discussion_r2606738940
##
superset-frontend/plugins/plugin-chart-table/src/DataTable/types/react-table.d.ts:
##
@@ -93,8 +96,7 @@ declare module 'react-table' {
}
export interface ColumnInterface
-extends UseGlobalFiltersColumnOptions,
- UseSortByColumnOptions {
+extends UseGlobalFiltersColumnOptions, UseSortByColumnOptions {
Review Comment:
**Suggestion:** The `ColumnInterface` was extended with
`UseGlobalFiltersColumnOptions`, which is not imported in this file and will
cause a TypeScript unresolved type error; remove the unreferenced type (or
import the correct column-level global filter type) — minimal fix shown removes
the unimported symbol and keeps the already-imported `UseSortByColumnOptions`.
[type error]
**Severity Level:** Minor ⚠️
```suggestion
extends UseSortByColumnOptions {
```
Why it matters? ⭐
This is a valid, actionable fix: UseGlobalFiltersColumnOptions isn't
imported and likely wasn't intended here. Dropping it and keeping
UseSortByColumnOptions fixes the unresolved type while preserving sort-related
typings.
Prompt for AI Agent 🤖
```mdx
This is a comment left during a code review.
**Path:**
superset-frontend/plugins/plugin-chart-table/src/DataTable/types/react-table.d.ts
**Line:** 99:99
**Comment:**
*Type Error: The `ColumnInterface` was extended with
`UseGlobalFiltersColumnOptions`, which is not imported in this file and will
cause a TypeScript unresolved type error; remove the unreferenced type (or
import the correct column-level global filter type) — minimal fix shown removes
the unimported symbol and keeps the already-imported `UseSortByColumnOptions`.
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.
```
##
superset-frontend/plugins/legacy-plugin-chart-country-map/src/transformProps.js:
##
@@ -24,8 +24,31 @@ export default function transformProps(chartProps) {
selectCountry,
colorScheme,
sliceId,
+customColorRules,
+customColorScale,
} = formData;
+ let parsedColorRules = [];
+ let parsedColorScale = [];
+
+ try {
+parsedColorRules = customColorRules ? JSON.parse(customColorRules) : [];
+ } catch (error) {
+console.warn(
+ 'Invalid JSON in customColorRules. Please check your configuration
syntax:',
+ error && error.message ? error.message : error,
+);
Review Comment:
**Suggestion:** Passing non-string values (objects/arrays) to JSON.parse
will throw a runtime error or a SyntaxError; ensure you only call JSON.parse on
strings and otherwise accept already-parsed objects/arrays or fallback to an
empty array. Also explicitly reset `parsedColorRules` in the catch so its value
is predictable after a failed parse. [type error]
**Severity Level:** Minor ⚠️
```suggestion
if (!customColorRules) {
parsedColorRules = [];
} else if (typeof customColorRules === 'string') {
parsedColorRules = JSON.parse(customColorRules);
} else {
// already an object/array — accept as-is
parsedColorRules = customColorRules;
}
} catch (error) {
console.warn(
'Invalid JSON in customColorRules. Please check your configuration
syntax:',
error && error.message ? error.message : error,
);
parsedColorRules = [];
```
Why it matters? ⭐
The current code unconditionally calls JSON.parse when a value exists — if
the caller already passes an object/array, JSON.parse will throw. Guarding on
typeof and accepting already-parsed objects avoids unnecessary runtime errors
and makes the behavior predictable. Resetting to [] in the catch also makes the
post-error state explicit (though parsedColorRules is initialized to []
already).
Prompt for AI Agent 🤖
```mdx
This is a comment left during a code review.
**Path:**
superset-frontend/plugins/legacy-plugin-chart-country-map/src/transformProps.js
**Line:** 35:40
**Comment:**
*Type Error: Passing non-string values (objects/arrays) to JSON.parse
will throw a runtime error or a SyntaxError; ensure you only call JSON.parse on
strings and otherwise accept already-parsed objects/arrays or fallback to an
empty array. Also explicitly reset `parsedColorRules` in the catch so its value
is predictable after a failed parse.
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.
```
##
superset-frontend/plugins/legacy-plugin-chart-country-map/src/CountryMap.js:
##
@@ -41,10 +64,46 @@ const propTypes = {
linearColorScheme: PropTypes.string,
mapBase
Re: [PR] feat: add custom colors and thresholds for CountryMap plugin [superset]
codeant-ai-for-open-source[bot] commented on PR #36103: URL: https://github.com/apache/superset/pull/36103#issuecomment-3637185873 ## 💡 Enhance Your PR Reviews We noticed that **3 feature(s)** are not configured for this repository. Enabling these features can help improve your code quality and workflow: ### 🚦 Quality Gates **Status:** Quality Gates are not enabled at the organization level Learn more about [Quality Gates](https://docs.codeant.ai/quality-gates) ### 🎫 Jira Ticket Compliance **Status:** Jira credentials file not found. Please configure Jira integration in your settings Learn more about [Jira Integration](https://docs.codeant.ai/integrations/jira) ### ⚙️ Custom Rules **Status:** No custom rules configured. Add rules via organization settings or .codeant/review.json in your repository Learn more about [Custom Rules](https://docs.codeant.ai/custom-rules) --- **Want to enable these features?** Contact your organization admin or check our documentation for setup instructions. -- 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]
Re: [PR] feat: add custom colors and thresholds for CountryMap plugin [superset]
codeant-ai-for-open-source[bot] commented on PR #36103: URL: https://github.com/apache/superset/pull/36103#issuecomment-3637185714 CodeAnt AI finished reviewing your PR. -- 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]
Re: [PR] feat: add custom colors and thresholds for CountryMap plugin [superset]
codeant-ai-for-open-source[bot] commented on PR #36103: URL: https://github.com/apache/superset/pull/36103#issuecomment-3637136217 CodeAnt AI is reviewing your PR. -- 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]
Re: [PR] feat: add custom colors and thresholds for CountryMap plugin [superset]
Facyla commented on PR #36103: URL: https://github.com/apache/superset/pull/36103#issuecomment-3636510777 Hello @rusackas, I see that there are ongoing updates (https://github.com/apache/superset/commit/67cf287c033cd88c59695bb4337478a36dcb1a27) on the country map plugin with the rc4 release, and wonder what would be the best way to integrate this in the upcoming 6.0.0 release: I am wondering whether I should keep updating this PR, or rather create on a new one on rc4 base code? (latest seems easier than reconcialiating changes, especially after latest UI additions) -- 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]
Re: [PR] feat: add custom colors and thresholds for CountryMap plugin [superset]
Facyla commented on PR #36103: URL: https://github.com/apache/superset/pull/36103#issuecomment-3617886731 Thank you @rusackas i installed and setup pre-commit for the project, and should update the PR in the beginning of the week -- 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]
Re: [PR] feat: add custom colors and thresholds for CountryMap plugin [superset]
rusackas commented on PR #36103: URL: https://github.com/apache/superset/pull/36103#issuecomment-3614433834 Looks like pre-commit might clear up some of the CI issues. -- 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]
Re: [PR] feat: add custom colors and thresholds for CountryMap plugin [superset]
Facyla commented on PR #36103: URL: https://github.com/apache/superset/pull/36103#issuecomment-3576740169 Hi, Latest commit fixes an unwanted behaviour, which would interpolate colors between the custom defined steps. Also removed a feature that added default colors to zones without data - which should remain white and blank. And a small french typo. Thank you -- 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]
Re: [PR] feat: add custom colors and thresholds for CountryMap plugin [superset]
Facyla commented on PR #36103: URL: https://github.com/apache/superset/pull/36103#issuecomment-3576686461 Latest commit fixes an unwanted interpolation issue that would create scales between custom-defined steps (while we want ranges with the same color instead). -- 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]
Re: [PR] feat: add custom colors and thresholds for CountryMap plugin [superset]
Facyla commented on code in PR #36103:
URL: https://github.com/apache/superset/pull/36103#discussion_r2545703660
##
superset-frontend/plugins/legacy-plugin-chart-country-map/src/CountryMap.js:
##
@@ -27,6 +27,15 @@ import {
} from '@superset-ui/core';
import countries, { countryOptions } from './countries';
+function normalizeColorKeyword(color) {
+ if (!color && color !== '') return '#00';
+ const c = String(color).trim().toLowerCase();
+ if (/^#([0-9a-f]{3}|[0-9a-f]{6})$/i.test(c)) return c;
+ if (/^[a-z]+$/.test(c)) return c;
+
+ return '#00';
+}
Review Comment:
I'll handle all valid css colors
--
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]
Re: [PR] feat: add custom colors and thresholds for CountryMap plugin [superset]
Copilot commented on code in PR #36103:
URL: https://github.com/apache/superset/pull/36103#discussion_r2535114631
##
superset-frontend/plugins/legacy-plugin-chart-country-map/src/CountryMap.js:
##
@@ -53,24 +98,208 @@ function CountryMap(element, props) {
country,
linearColorScheme,
numberFormat,
+customColorRules = [],
+customColorScale = [],
+minColor,
+maxColor,
colorScheme,
sliceId,
} = props;
+ const minColorHexRaw = rgbaToHex(minColor) || '#f7fbff';
+ const maxColorHexRaw = rgbaToHex(maxColor) || '#08306b';
+ const minColorHex = normalizeColorKeyword(minColorHexRaw);
+ const maxColorHex = normalizeColorKeyword(maxColorHexRaw);
+
const container = element;
const format = getNumberFormatter(numberFormat);
+ const normalizedScale = normalizeScale(customColorScale);
+ const normalizedScaleWithColors = Array.isArray(normalizedScale)
+? normalizedScale.map(e => {
+if (!e || typeof e !== 'object') return e;
+return { ...e, color: normalizeColorKeyword(e.color) };
+ })
+: [];
+
const linearColorScale = getSequentialSchemeRegistry()
.get(linearColorScheme)
.createLinearScale(d3Extent(data, v => v.metric));
const colorScale = CategoricalColorNamespace.getScale(colorScheme);
+ // Parse metrics to numbers safely
+ const parsedData = Array.isArray(data)
+? data.map(r => ({ ...r, metric: safeNumber(r.metric) }))
+: [];
+
+ // numeric values only
+ const numericValues = parsedData
+.map(r => r.metric)
+.filter(v => Number.isFinite(v));
+
+ let minValue = 0;
+ let maxValue = 1;
+ if (numericValues.length > 0) {
+const extent = d3Extent(numericValues);
+minValue = extent[0];
+maxValue = extent[1];
+ }
+
+ const valueRange = maxValue - minValue;
+ const valueRangeNonZero = valueRange === 0 ? 1 : valueRange;
+
+ /** -
+ * 1) Custom conditional rules
+ * - */
+ const getColorFromRules = value => {
+if (!Array.isArray(customColorRules) || !Number.isFinite(value))
+ return null;
+for (const rule of customColorRules) {
+ if (
+rule &&
+typeof rule.color === 'string' &&
+(('min' in rule && 'max' in rule) || 'value' in rule)
+ ) {
+if ('value' in rule && Number(rule.value) === value) {
+ // 🆕 normalize possible keyword in conditional rules as well
+ return normalizeColorKeyword(rule.color);
+}
+if ('min' in rule && 'max' in rule) {
+ const minR = safeNumber(rule.min);
+ const maxR = safeNumber(rule.max);
+ if (
+Number.isFinite(minR) &&
+Number.isFinite(maxR) &&
+value >= minR &&
+value <= maxR
+ ) {
+return normalizeColorKeyword(rule.color);
+ }
+}
+ }
+}
+return null;
+ };
+
+ /** -
+ * 2) Custom color scale (by %)
+ * - */
+ let percentColorScale = null;
+ if (
+Array.isArray(normalizedScaleWithColors) &&
+normalizedScaleWithColors.length >= 2
+ ) {
+const sorted = normalizedScaleWithColors
+ .filter(
+e => e && typeof e.percent === 'number' && typeof e.color === 'string',
+ )
+ .slice()
+ .sort((a, b) => a.percent - b.percent);
+
+if (sorted.length >= 2) {
+ const domainPerc = sorted.map(e => e.percent);
+ const rangeColors = sorted.map(e => e.color);
+ percentColorScale = d3.scale
+.linear()
+.domain(domainPerc)
+.range(rangeColors)
+.clamp(true)
+.interpolate(d3.interpolateRgb);
+}
+ }
+
+ /** -
+ * 3) Linear palette from registry (SI défini)
+ * - */
+ let linearPaletteScale = null;
+ if (linearColorScheme) {
+try {
+ const seq = getSequentialSchemeRegistry().get(linearColorScheme);
+ if (seq && typeof seq.createLinearScale === 'function') {
+linearPaletteScale = seq.createLinearScale([minValue, maxValue]);
+ } else if (seq && Array.isArray(seq.colors) && seq.colors.length >= 2) {
+linearPaletteScale = d3.scale
+ .linear()
+ .domain([minValue, maxValue])
+ .range([seq.colors[0], seq.colors[seq.colors.length - 1]])
+ .interpolate(d3.interpolateRgb);
+ }
+} catch {
+ linearPaletteScale = null;
+}
+ }
+
+ /** -
+ * 4) Gradient fallback (minColor → maxColor) avec HEX
+ * - */
+ let gradientColorScale;
+ if (minValue === maxValue) {
+gradientColorScale = () => minColorHex || maxColorHex || '#ddd';
+ } else {
+gradientColorScale = d3.scale
+ .linear()
+ .domain([minValue, maxValue])
+ .range([minColorHex, maxColorHex])
+ .interpolate(d3.interpolateRgb);
+ }
+
+ /** -
+ * Build final color (
Re: [PR] feat: add custom colors and thresholds for CountryMap plugin [superset]
bito-code-review[bot] commented on PR #36103: URL: https://github.com/apache/superset/pull/36103#issuecomment-3529110292 AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR). -- 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]
