Copilot commented on code in PR #38349:
URL: https://github.com/apache/superset/pull/38349#discussion_r2874397323
##########
superset-frontend/src/dashboard/components/gridComponents/Chart/Chart.tsx:
##########
@@ -761,6 +761,7 @@ const Chart = (props: ChartProps) => {
emitCrossFilters={emitCrossFilters}
onChartStateChange={handleChartStateChange}
suppressLoadingSpinner={suppressLoadingSpinner}
+ filterState={dataMask[props.id]?.filterState}
Review Comment:
This regression fix would be safer with a focused unit test asserting that
the dashboard grid `Chart` passes `dataMask[chartId].filterState` down to the
rendered chart (e.g., by mocking/inspecting `ChartContainer`/`ChartRenderer`
props). That would prevent future TSX refactors from accidentally dropping the
prop again.
##########
superset-frontend/src/dashboard/components/gridComponents/Chart/Chart.tsx:
##########
@@ -761,6 +761,7 @@ const Chart = (props: ChartProps) => {
emitCrossFilters={emitCrossFilters}
onChartStateChange={handleChartStateChange}
suppressLoadingSpinner={suppressLoadingSpinner}
+ filterState={dataMask[props.id]?.filterState}
Review Comment:
`filterState` is being passed into `ChartContainer`, but the connected
component ultimately renders `src/components/Chart/Chart` whose `ChartProps`
interface does not declare a `filterState` prop. This is likely to cause a
TypeScript type error (and it also leaves the prop out of the public contract
even though `ChartRenderer`/`SuperChart` support it). Add `filterState?:
FilterState` to the chart component props (and ensure it’s forwarded) so this
remains type-safe.
--
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]
