From aeb3081624588ff0d4c3500cd223c58327225439 Mon Sep 17 00:00:00 2001
From: Thomas Heartman <thomas@getunleash.io>
Date: Thu, 20 Mar 2025 13:54:19 +0100
Subject: [PATCH] chore: Don't use fallback functions for dragging (#9585)

Makes it so that strategies project env strategies that aren't draggable
don't get the drag icon. The reason it didn't work as expected was that
we used fallback functions instead of keeping them undefined.

I discovered that we applied two dragging boxes, so I removed the outer
layer one (specific to project envs) in favor of relying on the inner
one. Most of the lines changed are just indentation as a result of this
nesting going away.

Here's the diff. The top set of strategies aren't draggable; the lower
ones are.


![image](https://github.com/user-attachments/assets/0a7b6371-9f34-4596-a85f-9881da821448)
---
 ...rojectEnvironmentStrategyDraggableItem.tsx | 118 ++++++++----------
 .../StrategyDraggableItem.tsx                 |  12 +-
 2 files changed, 59 insertions(+), 71 deletions(-)

diff --git a/frontend/src/component/feature/FeatureView/FeatureOverview/FeatureOverviewEnvironments/FeatureOverviewEnvironment/EnvironmentAccordionBody/StrategyDraggableItem/ProjectEnvironmentStrategyDraggableItem.tsx b/frontend/src/component/feature/FeatureView/FeatureOverview/FeatureOverviewEnvironments/FeatureOverviewEnvironment/EnvironmentAccordionBody/StrategyDraggableItem/ProjectEnvironmentStrategyDraggableItem.tsx
index 19de8335cf..cd0a413dee 100644
--- a/frontend/src/component/feature/FeatureView/FeatureOverview/FeatureOverviewEnvironments/FeatureOverviewEnvironment/EnvironmentAccordionBody/StrategyDraggableItem/ProjectEnvironmentStrategyDraggableItem.tsx
+++ b/frontend/src/component/feature/FeatureView/FeatureOverview/FeatureOverviewEnvironments/FeatureOverviewEnvironment/EnvironmentAccordionBody/StrategyDraggableItem/ProjectEnvironmentStrategyDraggableItem.tsx
@@ -1,5 +1,5 @@
 import { type DragEventHandler, type RefObject, useRef } from 'react';
-import { Box, useMediaQuery, useTheme } from '@mui/material';
+import { useMediaQuery, useTheme } from '@mui/material';
 import type { IFeatureEnvironment } from 'interfaces/featureToggle';
 import type { IFeatureStrategy } from 'interfaces/strategy';
 import { useRequiredPathParam } from 'hooks/useRequiredPathParam';
@@ -34,8 +34,6 @@ type ProjectEnvironmentStrategyDraggableItemProps = {
     onDragEnd?: () => void;
 };
 
-const onDragNoOp = () => () => {};
-
 export const ProjectEnvironmentStrategyDraggableItem = ({
     className,
     strategy,
@@ -43,9 +41,9 @@ export const ProjectEnvironmentStrategyDraggableItem = ({
     environmentName,
     otherEnvironments,
     isDragging,
-    onDragStartRef = onDragNoOp,
-    onDragOver = onDragNoOp,
-    onDragEnd = onDragNoOp,
+    onDragStartRef,
+    onDragOver,
+    onDragEnd,
 }: ProjectEnvironmentStrategyDraggableItemProps) => {
     const projectId = useRequiredPathParam('projectId');
     const featureId = useRequiredPathParam('featureId');
@@ -75,67 +73,59 @@ export const ProjectEnvironmentStrategyDraggableItem = ({
     const isSmallScreen = useMediaQuery(theme.breakpoints.down('sm'));
 
     return (
-        <Box
-            className={className}
-            key={strategy.id}
-            ref={ref}
-            onDragOver={onDragOver(ref, index)}
-            sx={{ opacity: isDragging ? '0.5' : '1' }}
-        >
-            <StrategyDraggableItem
-                strategy={strategy}
-                onDragEnd={onDragEnd}
-                onDragStartRef={onDragStartRef}
-                onDragOver={onDragOver}
-                index={index}
-                isDragging={isDragging}
-                headerItemsRight={
-                    <>
-                        {draftChange && !isSmallScreen ? (
-                            <ChangeRequestDraftStatusBadge
-                                sx={{ mr: 1.5 }}
-                                changeAction={draftChange.change.action}
-                            />
-                        ) : null}
+        <StrategyDraggableItem
+            strategy={strategy}
+            onDragEnd={onDragEnd}
+            onDragStartRef={onDragStartRef}
+            onDragOver={onDragOver}
+            index={index}
+            isDragging={isDragging}
+            headerItemsRight={
+                <>
+                    {draftChange && !isSmallScreen ? (
+                        <ChangeRequestDraftStatusBadge
+                            sx={{ mr: 1.5 }}
+                            changeAction={draftChange.change.action}
+                        />
+                    ) : null}
 
-                        {scheduledChanges &&
-                        scheduledChanges.length > 0 &&
-                        !isSmallScreen ? (
-                            <ChangesScheduledBadge
-                                scheduledChangeRequestIds={(
-                                    scheduledChanges ?? []
-                                ).map((scheduledChange) => scheduledChange.id)}
-                            />
-                        ) : null}
-                        {otherEnvironments && otherEnvironments?.length > 0 ? (
-                            <CopyStrategyIconMenu
-                                environmentId={environmentName}
-                                environments={otherEnvironments as string[]}
-                                strategy={strategy}
-                            />
-                        ) : null}
-                        <PermissionIconButton
-                            permission={UPDATE_FEATURE_STRATEGY}
-                            environmentId={environmentName}
-                            projectId={projectId}
-                            component={Link}
-                            to={editStrategyPath}
-                            tooltipProps={{
-                                title: 'Edit strategy',
-                            }}
-                            data-testid={`STRATEGY_EDIT-${strategy.name}`}
-                        >
-                            <Edit />
-                        </PermissionIconButton>
-                        <MenuStrategyRemove
-                            projectId={projectId}
-                            featureId={featureId}
+                    {scheduledChanges &&
+                    scheduledChanges.length > 0 &&
+                    !isSmallScreen ? (
+                        <ChangesScheduledBadge
+                            scheduledChangeRequestIds={(
+                                scheduledChanges ?? []
+                            ).map((scheduledChange) => scheduledChange.id)}
+                        />
+                    ) : null}
+                    {otherEnvironments && otherEnvironments?.length > 0 ? (
+                        <CopyStrategyIconMenu
                             environmentId={environmentName}
+                            environments={otherEnvironments as string[]}
                             strategy={strategy}
                         />
-                    </>
-                }
-            />
-        </Box>
+                    ) : null}
+                    <PermissionIconButton
+                        permission={UPDATE_FEATURE_STRATEGY}
+                        environmentId={environmentName}
+                        projectId={projectId}
+                        component={Link}
+                        to={editStrategyPath}
+                        tooltipProps={{
+                            title: 'Edit strategy',
+                        }}
+                        data-testid={`STRATEGY_EDIT-${strategy.name}`}
+                    >
+                        <Edit />
+                    </PermissionIconButton>
+                    <MenuStrategyRemove
+                        projectId={projectId}
+                        featureId={featureId}
+                        environmentId={environmentName}
+                        strategy={strategy}
+                    />
+                </>
+            }
+        />
     );
 };
diff --git a/frontend/src/component/feature/FeatureView/FeatureOverview/FeatureOverviewEnvironments/FeatureOverviewEnvironment/EnvironmentAccordionBody/StrategyDraggableItem/StrategyDraggableItem.tsx b/frontend/src/component/feature/FeatureView/FeatureOverview/FeatureOverviewEnvironments/FeatureOverviewEnvironment/EnvironmentAccordionBody/StrategyDraggableItem/StrategyDraggableItem.tsx
index 0ca7ca554a..6e2d857c6f 100644
--- a/frontend/src/component/feature/FeatureView/FeatureOverview/FeatureOverviewEnvironments/FeatureOverviewEnvironment/EnvironmentAccordionBody/StrategyDraggableItem/StrategyDraggableItem.tsx
+++ b/frontend/src/component/feature/FeatureView/FeatureOverview/FeatureOverviewEnvironments/FeatureOverviewEnvironment/EnvironmentAccordionBody/StrategyDraggableItem/StrategyDraggableItem.tsx
@@ -8,8 +8,6 @@ import { Box } from '@mui/material';
 import type { IFeatureStrategy } from 'interfaces/strategy';
 import { StrategyItem } from './StrategyItem/StrategyItem';
 
-const onDragNoOp = () => () => {};
-
 type StrategyDraggableItemProps = {
     headerItemsRight: ReactNode;
     strategy: IFeatureStrategy;
@@ -30,9 +28,9 @@ export const StrategyDraggableItem = ({
     strategy,
     index,
     isDragging,
-    onDragStartRef = onDragNoOp,
-    onDragOver = onDragNoOp,
-    onDragEnd = onDragNoOp,
+    onDragStartRef,
+    onDragOver,
+    onDragEnd,
     headerItemsRight,
 }: StrategyDraggableItemProps) => {
     const ref = useRef<HTMLDivElement>(null);
@@ -41,13 +39,13 @@ export const StrategyDraggableItem = ({
         <Box
             key={strategy.id}
             ref={ref}
-            onDragOver={onDragOver(ref, index)}
+            onDragOver={onDragOver?.(ref, index)}
             sx={{ opacity: isDragging ? '0.5' : '1' }}
         >
             <StrategyItem
                 headerItemsRight={headerItemsRight}
                 strategy={strategy}
-                onDragStart={onDragStartRef(ref, index)}
+                onDragStart={onDragStartRef?.(ref, index)}
                 onDragEnd={onDragEnd}
             />
         </Box>