korbit-ai[bot] commented on code in PR #34244:
URL: https://github.com/apache/superset/pull/34244#discussion_r2218649518
##########
superset-frontend/src/explore/components/controls/ColorBreakpointsControl/ColorBreakpointPopoverControl.tsx:
##########
@@ -76,7 +76,7 @@ const determineErrorMap = (
return errorMap;
}
- if (newMinValue >= newMaxValue) {
+ if (newMinValue > newMaxValue) {
errorMap.minValue.push(
t('Min value should be smaller or equal to max value'),
);
Review Comment:
### Inconsistent error message with validation logic <sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
The error message indicates that the min value should be 'smaller or equal'
but the code logic only checks for 'smaller than', creating inconsistency
between documentation and implementation.
###### Why this matters
This inconsistency could confuse users when equal values are accepted but
the error message suggests otherwise.
###### Suggested change ∙ *Feature Preview*
Change the error message to: t('Min value should be smaller than max value')
###### Provide feedback to improve future suggestions
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/9bff6221-675f-4cfe-8ac4-a20c0e254f01/upvote)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/9bff6221-675f-4cfe-8ac4-a20c0e254f01?what_not_true=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/9bff6221-675f-4cfe-8ac4-a20c0e254f01?what_out_of_scope=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/9bff6221-675f-4cfe-8ac4-a20c0e254f01?what_not_in_standard=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/9bff6221-675f-4cfe-8ac4-a20c0e254f01)
</details>
<sub>
💬 Looking for more details? Reply to this comment to chat with Korbit.
</sub>
<!--- korbi internal id:6f644eb6-c08f-4b97-b75b-8e5796d606d0 -->
[](6f644eb6-c08f-4b97-b75b-8e5796d606d0)
##########
superset-frontend/plugins/legacy-preset-chart-deckgl/src/layers/Hex/controlPanel.ts:
##########
@@ -53,7 +54,10 @@ const config: ControlPanelConfig = {
label: t('Map'),
controlSetRows: [
[mapboxStyle],
- ...generateDeckGLColorSchemeControls({}),
+ ...generateDeckGLColorSchemeControls({
+ defaultSchemeType: COLOR_SCHEME_TYPES.categorical_palette,
Review Comment:
### Incorrect Color Scheme Type for Heatmap <sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
Using a categorical color palette for a hexagonal heatmap visualization is
incorrect. Hexagonal heatmaps require sequential or diverging color schemes as
they represent continuous numerical data.
###### Why this matters
Categorical color schemes are designed for discrete, unordered data
categories. Using them for continuous numerical data in a heatmap will result
in poor data visualization and could mislead users interpreting the patterns in
the data.
###### Suggested change ∙ *Feature Preview*
Change the defaultSchemeType to use a sequential color scheme:
```typescript
...generateDeckGLColorSchemeControls({
defaultSchemeType: COLOR_SCHEME_TYPES.sequential,
disableCategoricalColumn: true,
})
```
###### Provide feedback to improve future suggestions
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/3f11f536-6f98-4dae-8426-04c3734ef9af/upvote)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/3f11f536-6f98-4dae-8426-04c3734ef9af?what_not_true=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/3f11f536-6f98-4dae-8426-04c3734ef9af?what_out_of_scope=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/3f11f536-6f98-4dae-8426-04c3734ef9af?what_not_in_standard=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/3f11f536-6f98-4dae-8426-04c3734ef9af)
</details>
<sub>
💬 Looking for more details? Reply to this comment to chat with Korbit.
</sub>
<!--- korbi internal id:8b728a52-4596-4f03-9693-dc653e31e807 -->
[](8b728a52-4596-4f03-9693-dc653e31e807)
##########
superset-frontend/plugins/legacy-preset-chart-deckgl/src/utilities/utils.ts:
##########
@@ -37,4 +37,4 @@ export function formatSelectOptions(options: (string |
number)[]) {
export const isColorSchemeTypeVisible = (
controls: ControlStateMapping,
colorSchemeType: ColorSchemeType,
-) => controls.color_scheme_type.value === colorSchemeType;
+) => controls.color_scheme_type?.value === colorSchemeType;
Review Comment:
### Incorrect Color Scheme Type Comparison <sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
The optional chaining operator (?.) can result in undefined being compared
to colorSchemeType when controls.color_scheme_type is undefined, which is
likely not the intended behavior.
###### Why this matters
When controls.color_scheme_type is undefined, the function will return false
regardless of the colorSchemeType value, potentially hiding color scheme
options that should be visible.
###### Suggested change ∙ *Feature Preview*
Add a default value to handle undefined cases appropriately:
```typescript
export const isColorSchemeTypeVisible = (
controls: ControlStateMapping,
colorSchemeType: ColorSchemeType,
) => (controls.color_scheme_type?.value ?? 'fixed_color') ===
colorSchemeType;
```
###### Provide feedback to improve future suggestions
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/518a9341-b8da-4d9f-a239-253044f7f62b/upvote)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/518a9341-b8da-4d9f-a239-253044f7f62b?what_not_true=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/518a9341-b8da-4d9f-a239-253044f7f62b?what_out_of_scope=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/518a9341-b8da-4d9f-a239-253044f7f62b?what_not_in_standard=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/518a9341-b8da-4d9f-a239-253044f7f62b)
</details>
<sub>
💬 Looking for more details? Reply to this comment to chat with Korbit.
</sub>
<!--- korbi internal id:bfdf8271-8354-415a-98c7-c9e79f68a9a2 -->
[](bfdf8271-8354-415a-98c7-c9e79f68a9a2)
--
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]
