sadpandajoe commented on code in PR #41132:
URL: https://github.com/apache/superset/pull/41132#discussion_r3462974366
##########
superset-frontend/plugins/plugin-chart-echarts/src/BigNumber/BigNumberViz.tsx:
##########
@@ -43,14 +42,11 @@ function BigNumberVis({
kickerFontSize = PROPORTION.KICKER,
metricNameFontSize = PROPORTION.METRIC_NAME,
showMetricName = true,
- mainColor = BRAND_COLOR,
showTimestamp = false,
showTrendLine = false,
- startYAxisAtZero = true,
subheader = '',
subheaderFontSize = PROPORTION.SUBHEADER,
subtitleFontSize = PROPORTION.SUBHEADER,
- timeRangeFixed = false,
...props
}: BigNumberVizProps) {
Review Comment:
`mainColor`, `startYAxisAtZero`, and `timeRangeFixed` were removed from this
destructuring, but `BigNumberVizProps` still declares all three as optional
fields (types.ts:94-98). Callers that pass them compile fine and have the
values silently dropped instead of getting a compile error, and `mainColor`
reads like it does something. Can you remove the three fields from types.ts too
so the dead surface goes away completely?
##########
superset-frontend/src/dashboard/components/Dashboard.tsx:
##########
@@ -114,12 +114,8 @@ function Dashboard({
slices,
activeFilters,
chartConfiguration,
- datasources,
ownDataCharts,
layout,
- impressionId,
- timeout = 60,
- userId = '',
children,
}: DashboardProps): JSX.Element {
Review Comment:
Same pattern as BigNumberViz: `datasources`, `impressionId`, `timeout`, and
`userId` are gone from this destructuring but `DashboardProps` still exposes
them just above (lines 79-84), so callers can keep passing them and the values
are silently ignored. `datasources` being a no-op on a Dashboard component is
the confusing one. Worth trimming these from the props type too.
--
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]
