Copilot commented on code in PR #37207:
URL: https://github.com/apache/superset/pull/37207#discussion_r2713884428
##########
superset-frontend/plugins/plugin-chart-echarts/src/Sunburst/transformProps.ts:
##########
@@ -380,7 +371,7 @@ export default function transformProps(
data: traverse(treeData, []),
},
],
- graphic: showTotal
+ graphic: showTotal
Review Comment:
This indentation is inconsistent with the surrounding code. The `graphic:`
property should be aligned with the other properties in the echartOptions
object (such as `grid`, `tooltip`, and `series`), not indented further to the
right.
```suggestion
graphic: showTotal
```
##########
superset-frontend/plugins/plugin-chart-echarts/src/Sunburst/transformProps.ts:
##########
@@ -389,11 +380,12 @@ export default function transformProps(
text: t('Total: %s', primaryValueFormatter(totalValue)),
fontSize: 16,
fontWeight: 'bold',
- fill: theme.colorText,
+ color: theme.colorText,
Review Comment:
The property should be `fill` instead of `color` for ECharts graphic text
elements. The Pie chart implementation (which has the same "Show Total"
feature) uses `fill: theme.colorText` at line 463 of Pie/transformProps.ts.
Using `color` instead of `fill` may not work correctly with ECharts' graphic
text API, which follows SVG conventions.
```suggestion
fill: theme.colorText,
```
##########
superset/config.py:
##########
@@ -794,7 +794,6 @@ class D3TimeFormat(TypedDict, total=False):
"colorInfo": "#66bcfe",
# Fonts
"fontUrls": [],
Review Comment:
The removal of the fontFamily token appears to be unrelated to the PR's
stated purpose of fixing Sunburst chart text visibility in dark mode. This is a
breaking change that could affect any themes or configurations that depend on
this token. The fontFamily property is used throughout the codebase (e.g., in
GlobalStyles.tsx, ThemedAgGridReact, and various chart components), and
removing it from the default theme configuration may cause issues.
```suggestion
"fontUrls": [],
"fontFamily": "-apple-system, BlinkMacSystemFont, 'Segoe UI',
sans-serif",
```
--
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]
