codeant-ai-for-open-source[bot] commented on code in PR #36932:
URL: https://github.com/apache/superset/pull/36932#discussion_r2666473745
##########
superset-frontend/packages/superset-ui-core/src/components/AsyncAceEditor/index.tsx:
##########
@@ -294,7 +294,7 @@ export function AsyncAceEditor(
}
/* Adjust selection color */
.ace_editor .ace_selection {
- background-color: ${token.colorPrimaryBgHover} !important;
+ background-color: ${token.colorEditorSelection ??
token.colorPrimaryBgHover} !important;
Review Comment:
**Suggestion:** Fallback chain is incomplete: if both
`token.colorEditorSelection` and `token.colorPrimaryBgHover` are undefined the
generated CSS will contain "background-color: undefined", producing invalid
CSS; add a third safe fallback (e.g. `token.colorBgContainer`) to ensure a
valid color is always emitted. [possible bug]
**Severity Level:** Critical 🚨
```suggestion
background-color: ${token.colorEditorSelection ??
token.colorPrimaryBgHover ?? token.colorBgContainer} !important;
```
<details>
<summary><b>Why it matters? ⭐ </b></summary>
Adding a third fallback (token.colorBgContainer) is a harmless defensive
change that prevents emitting "background-color: undefined" if both tokens are
absent. The file already uses token.colorBgContainer elsewhere, so it's a
sensible default and fixes a potential rendering bug without side effects.
</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/AsyncAceEditor/index.tsx
**Line:** 297:297
**Comment:**
*Possible Bug: Fallback chain is incomplete: if both
`token.colorEditorSelection` and `token.colorPrimaryBgHover` are undefined the
generated CSS will contain "background-color: undefined", producing invalid
CSS; add a third safe fallback (e.g. `token.colorBgContainer`) to ensure a
valid color is always emitted.
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>
##########
superset-frontend/packages/superset-core/src/ui/theme/types.ts:
##########
@@ -159,6 +159,13 @@ export interface SupersetSpecificTokens {
echartsOptionsOverridesByChartType?: {
[chartType: string]: any;
};
+
+ // Editor-related
+ /**
+ * Background color for code editor text selection.
+ * Defaults to colorPrimaryBgHover if not specified.
Review Comment:
**Suggestion:** The JSDoc claims a default ("Defaults to
colorPrimaryBgHover") but this file only defines a TypeScript interface and
does not implement any runtime defaulting; consumers of this interface may not
actually receive that value. Either remove the misleading "Defaults to"
phrasing or clarify that consumers should explicitly fall back to
`colorPrimaryBgHover`. [possible bug]
**Severity Level:** Critical 🚨
```suggestion
* If unspecified, consumers should fall back to `colorPrimaryBgHover`; no
automatic default is applied here.
```
<details>
<summary><b>Why it matters? ⭐ </b></summary>
The JSDoc is currently misleading: a TS interface can't apply runtime
defaults.
Clarifying the comment prevents incorrect assumptions by consumers without
changing runtime behavior.
</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-core/src/ui/theme/types.ts
**Line:** 166:166
**Comment:**
*Possible Bug: The JSDoc claims a default ("Defaults to
colorPrimaryBgHover") but this file only defines a TypeScript interface and
does not implement any runtime defaulting; consumers of this interface may not
actually receive that value. Either remove the misleading "Defaults to"
phrasing or clarify that consumers should explicitly fall back to
`colorPrimaryBgHover`.
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]
