codeant-ai-for-open-source[bot] commented on code in PR #37293:
URL: https://github.com/apache/superset/pull/37293#discussion_r2709731820
##########
superset-frontend/packages/superset-ui-core/src/components/Label/index.tsx:
##########
@@ -37,7 +37,7 @@ export function Label(props: LabelProps) {
...rest
} = props;
- const baseColor = themeObject.getColorVariants(type);
+ const baseColor = getColorVariants(theme, type);
Review Comment:
**Suggestion:** The label now directly passes `type` into
`getColorVariants`, but elsewhere in the app `type` is sometimes set to
unsupported values like `'danger'` (cast to the `LabelType` union), which makes
`getColorVariants` look up non-existent theme keys such as `colorDangerActive`
and returns `undefined` colors, resulting in broken or invisible label styling;
normalize unsupported variants (e.g., map `'danger'` to `'error'`) before
calling `getColorVariants`. [logic error]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
- ❌ Many pages render labels with wrong styling (visual regressions).
- ⚠️ Affects AnnotationLayerList label displays
(src/pages/AnnotationLayerList).
- ⚠️ Affects DatasetList label displays (src/pages/DatasetList).
- ⚠️ Affects RowLevelSecurityList label displays
(src/pages/RowLevelSecurityList).
- ⚠️ Multiple other list pages using 'danger' labels.
```
</details>
```suggestion
const baseColor = getColorVariants(
theme,
(type as string) === 'danger' ? 'error' : type,
);
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Open a page that renders Label with type 'danger' (example: file
superset-frontend/src/pages/AnnotationLayerList/index.tsx line 334 where
an object uses
type: 'danger' — discovered via Grep).
2. That page renders the Label component exported from
packages/superset-ui-core/src/components/Label/index.tsx (Label
definition located at
packages/superset-ui-core/src/components/Label/index.tsx:26 per Grep).
3. At runtime Label receives props.type === 'danger' and executes the line
packages/superset-ui-core/src/components/Label/index.tsx:40 which calls
getColorVariants(theme, type).
4. Grep across the repo shows many call sites passing literal 'danger'
(pages such as
src/pages/AnnotationLayerList/index.tsx:334,
src/pages/RowLevelSecurityList/index.tsx:344,
src/pages/DatasetList/index.tsx:893, etc.). When 'danger' is passed,
getColorVariants
looks up theme keys for that variant; because the theme uses 'error'
variant names, the
lookup can return undefined fields (baseColor.active / .border / .bg).
5. The Label then assigns color = baseColor.active
(packages/.../Label/index.tsx:41). If
baseColor.active is undefined the rendered CSS gets undefined values and the
label styling
appears broken or inconsistent in the browser (visual regression on the
referenced pages).
6. Reproducing locally: run the app, navigate to any of the pages listed
(e.g.
AnnotationLayerList), inspect a rendered Label whose type prop is 'danger'
and verify
computed CSS color / background-color are missing or incorrect, confirming
the unresolved
variant lookup.
7. Reasoning: Grep evidence shows widespread usage of 'danger' as a Label
type.
Normalizing or mapping unsupported variants prior to calling
getColorVariants avoids
undefined lookups. If this suggestion is ignored, many existing pages using
'danger' will
continue to risk broken/inconsistent label styling.
```
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:**
superset-frontend/packages/superset-ui-core/src/components/Label/index.tsx
**Line:** 40:40
**Comment:**
*Logic Error: The label now directly passes `type` into
`getColorVariants`, but elsewhere in the app `type` is sometimes set to
unsupported values like `'danger'` (cast to the `LabelType` union), which makes
`getColorVariants` look up non-existent theme keys such as `colorDangerActive`
and returns `undefined` colors, resulting in broken or invisible label styling;
normalize unsupported variants (e.g., map `'danger'` to `'error'`) before
calling `getColorVariants`.
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]
