bito-code-review[bot] commented on code in PR #41132:
URL: https://github.com/apache/superset/pull/41132#discussion_r3538261765
##########
superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/hooks/useFilterOperations.ts:
##########
@@ -125,7 +125,7 @@ export function useFilterOperations({
);
const handleRearrangeFilters = useCallback(
- (dragIndex: number, targetIndex: number, id: string) => {
+ (dragIndex: number, targetIndex: number, _id: string) => {
Review Comment:
<!-- Bito Reply -->
The suggestion to remove the parameter is not appropriate in this case
because the function is used as a positional callback where the third argument
is required by the caller. Renaming the parameter to `_id` is the correct
approach to satisfy linting requirements while maintaining the necessary
function signature.
**superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/hooks/useFilterOperations.ts**
```
const handleRearrangeFilters = useCallback(
- (dragIndex: number, targetIndex: number, id: string) => {
+ (dragIndex: number, targetIndex: number, _id: string) => {
```
##########
superset-frontend/src/explore/components/ExploreAlert.tsx:
##########
@@ -51,7 +51,7 @@ export const ExploreAlert = forwardRef(
type = 'info',
className = '',
}: ControlPanelAlertProps,
- ref: RefObject<HTMLDivElement>,
+ _ref: RefObject<HTMLDivElement>,
Review Comment:
<!-- Bito Reply -->
The suggestion to remove `forwardRef` is valid if the component does not
actually need to expose a DOM ref to its parent. If the component is intended
to be a simple wrapper that does not require ref forwarding, removing
`forwardRef` simplifies the component contract and avoids misleading callers
into thinking a ref can be attached to it.
**superset-frontend/src/explore/components/ExploreAlert.tsx**
```
export const ExploreAlert = (
{
type = 'info',
className = '',
}: ControlPanelAlertProps) => {
```
##########
superset-frontend/packages/superset-ui-core/src/chart/components/Matrixify/MatrixifyGridCell.tsx:
##########
@@ -92,7 +92,7 @@ const MatrixNoDataComponent = () => {
* Individual grid cell component - memoized to prevent unnecessary re-renders
*/
const MatrixifyGridCell = memo(
- ({ cell, rowHeight, datasource, hooks }: MatrixifyGridCellProps) => {
+ ({ cell, hooks }: MatrixifyGridCellProps) => {
Review Comment:
<!-- Bito Reply -->
The reviewer is correct that `rowHeight` is still used in the memo
comparator, and removing it from the interface or the comparator would indeed
break re-renders when the height changes. Since you have already removed it
from the component's destructuring where it was unused, the current
implementation is correct and safe to keep as is.
**superset-frontend/packages/superset-ui-core/src/chart/components/Matrixify/MatrixifyGridCell.tsx**
```
const MatrixifyGridCell = memo(
- ({ cell, rowHeight, datasource, hooks }: MatrixifyGridCellProps) => {
+ ({ cell, hooks }: MatrixifyGridCellProps) => {
```
--
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]
