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]

Reply via email to