rusackas commented on PR #37902:
URL: https://github.com/apache/superset/pull/37902#issuecomment-3885899797
## Addressed Review Comments
Thanks for the detailed review! I've addressed all 12 issues raised:
1. **ChartDataProvider.tsx:151** - ✅ Fixed: Removed `handleFetchData` from
useEffect dependencies to prevent refetching when callbacks change
2. **reactify.tsx:101** - ✅ Fixed: Added `.call({ container:
containerRef.current })` to preserve legacy `this` context
3. **HorizonRow.tsx:73** - ✅ Fixed: Added `rawData.length > 0` guard for
empty array
4. **TTestTable.tsx:137-140** - ✅ Fixed: Added bounds checking with
`Math.min(control, data.length - 1)`
5. **TableRenderers.tsx:712-717** - ✅ Fixed: Changed dependency array to use
specific structural props instead of `props` object
6. **Chart.tsx:207** - ✅ Fixed: Initialized `renderStartTimeRef` with
`Logger.getTimestamp()`
7. **DatasourceEditor.tsx:909-985** - ✅ Fixed: Pass `validationErrors`
directly to callback instead of relying on async state
8. **Dashboard.tsx:101-103** - ✅ Fixed: Use `event` parameter instead of
`window.event`
9. **SliceAdder.tsx:190-233** - ✅ Fixed: Added refs to track latest `slices`
and `selectedSliceIdsSet` values
10. **Markdown.tsx:301-341** - ✅ Fixed: Added ErrorBoundary and
`handleRenderError` callback that sets `hasError` to true
11. **SaveModal.tsx:760** - ✅ Fixed: Added `isLoading` to disabled condition
for "Save & go to dashboard" button
12. **CollectionControl/index.tsx:86-206** - ✅ Fixed: Added rest spread
`...headerProps` to forward header props to ControlHeader
--
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]
