codeant-ai-for-open-source[bot] commented on code in PR #37207:
URL: https://github.com/apache/superset/pull/37207#discussion_r2697530263
##########
superset/config.py:
##########
@@ -794,7 +794,7 @@ class D3TimeFormat(TypedDict, total=False):
"colorInfo": "#66bcfe",
# Fonts
"fontUrls": [],
- "fontFamily": "Inter, Helvetica, Arial",
+ "fontFamily": "Inter, Helvetica, Arial, sans-serif",
Review Comment:
**Suggestion:** The change described in the PR (adding theme-aware color for
the Sunburst "Show Total" text) was supposed to be made in frontend TypeScript
(transformProps.ts), but the PR only adds a backend config token `fontFamily`
here — this does not implement the intended UI fix. Remove this backend-only
insertion and apply the actual color change in the frontend TypeScript file
(set text color from the theme, e.g., use theme.token.colorText in
transformProps.ts). [possible bug]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
- ❌ Sunburst "Show Total" remains invisible in dark theme.
- ⚠️ Frontend transformProps.ts still needs color change.
- ⚠️ Users in dark mode experience unreadable chart text.
```
</details>
```suggestion
# fontFamily should be configured in the frontend theme JSON / UI
code, not added as an ad-hoc backend token here
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Inspect backend theme token in superset/config.py around THEME_DEFAULT:
see token added
at line 797 (`"fontFamily": "Inter, Helvetica, Arial, sans-serif",`).
2. Build and run Superset server with this PR applied so backend provides
THEME_DEFAULT
including the new fontFamily token.
3. Open a dashboard containing a Sunburst chart that has the "Show Total"
text visible in
the chart UI (Sunburst UI is rendered client-side; the intended fix was in
frontend
transformProps.ts).
4. Observe "Show Total" text remains invisible/unchanged in dark mode
because the frontend
transformProps.ts (where the Sunburst text color must be set from
theme.token.colorText)
was not modified — the PR only added a backend token at
superset/config.py:797. This
demonstrates the UI fix was applied to the wrong layer.
```
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/config.py
**Line:** 797:797
**Comment:**
*Possible Bug: The change described in the PR (adding theme-aware color
for the Sunburst "Show Total" text) was supposed to be made in frontend
TypeScript (transformProps.ts), but the PR only adds a backend config token
`fontFamily` here — this does not implement the intended UI fix. Remove this
backend-only insertion and apply the actual color change in the frontend
TypeScript file (set text color from the theme, e.g., use theme.token.colorText
in transformProps.ts).
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]