refactor(dashboard): resolve drag-drop groups inside the state updater

useDashboardDragDrop no longer takes userDashboard or mirrors it in a
ref. reorderSections receives the source/destination group titles from
the drop event and looks the groups up inside the setUserDashboard
functional updater, where React hands it the freshest state. This drops
the ref, its sync effect, and the userDashboard parameter, and removes
the (practically unreachable) window where a drop could read a stale
dashboard between a state commit and the ref update. The destination
index is copied into a local so the updater stays pure under StrictMode.

Draft addressing review comment C6.1 — kept as its own commit for review.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Author: aldbr

packages/diracx-web-components/src/components/DashboardLayout/DashboardDrawer.tsx

@@ -75,7 +75,7 @@ export default function DashboardDrawer({
   const theme = useTheme();
 
   // Set up drag and drop
-  useDashboardDragDrop(userDashboard, setUserDashboard);
+  useDashboardDragDrop(setUserDashboard);
 
   const handleAppCreation = (appType: string) => {
     const group =

packages/diracx-web-components/src/components/DashboardLayout/useDashboardDragDrop.ts

@@ -1,6 +1,6 @@
 "use client";
 
-import { useEffect, useRef } from "react";
+import { useEffect } from "react";
 import { monitorForElements } from "@atlaskit/pragmatic-drag-and-drop/element/adapter";
 import { extractClosestEdge } from "@atlaskit/pragmatic-drag-and-drop-hitbox/closest-edge";
 import { DashboardGroup } from "../../types";
@@ -10,16 +10,8 @@ import { DashboardGroup } from "../../types";
  * Handles reordering items within and between groups.
  */
 export default function useDashboardDragDrop(
-  userDashboard: DashboardGroup[],
   setUserDashboard: React.Dispatch<React.SetStateAction<DashboardGroup[]>>,
 ) {
-  // Keep a ref to the latest dashboard so the monitor effect does not need
-  // to re-subscribe every time the dashboard changes.
-  const userDashboardRef = useRef(userDashboard);
-  useEffect(() => {
-    userDashboardRef.current = userDashboard;
-  }, [userDashboard]);
-
   useEffect(() => {
     return monitorForElements({
       onDrop({ source, location }) {
@@ -29,100 +21,85 @@ export default function useDashboardDragDrop(
         }
         const sourceData = source.data;
         const targetData = target.data;
-        const currentDashboard = userDashboardRef.current;
+        const sourceTitle = sourceData.title as string;
+        const destinationTitle = targetData.title as string;
+        const sourceIndex = sourceData.index as number;
 
         if (location.current.dropTargets.length === 2) {
-          const groupTitle = targetData.title;
           const closestEdgeOfTarget = extractClosestEdge(targetData);
           const targetIndex = targetData.index as number;
-          const sourceGroup = currentDashboard.find(
-            (group) => group.title === sourceData.title,
-          );
-          const targetGroup = currentDashboard.find(
-            (group) => group.title === groupTitle,
-          );
-          const sourceIndex = sourceData.index as number;
           const destinationIndex = (
             closestEdgeOfTarget === "top" ? targetIndex : targetIndex + 1
           ) as number;
 
           reorderSections(
-            sourceGroup,
-            targetGroup,
+            sourceTitle,
+            destinationTitle,
             sourceIndex,
             destinationIndex,
           );
         } else {
-          const groupTitle = targetData.title;
-          const sourceGroup = currentDashboard.find(
-            (group) => group.title === sourceData.title,
-          );
-          const targetGroup = currentDashboard.find(
-            (group) => group.title === groupTitle,
-          );
-          const sourceIndex = sourceData.index as number;
-
-          reorderSections(sourceGroup, targetGroup, sourceIndex);
+          reorderSections(sourceTitle, destinationTitle, sourceIndex);
         }
       },
     });
 
+    // The groups are looked up inside the functional updater, so it always
+    // sees the freshest state — no ref mirror, no re-subscription on every
+    // dashboard change, and no window where a drop reads a stale dashboard.
     function reorderSections(
-      sourceGroup: DashboardGroup | undefined,
-      destinationGroup: DashboardGroup | undefined,
+      sourceTitle: string,
+      destinationTitle: string,
       sourceIndex: number,
       destinationIndex: number | null = null,
     ) {
-      if (sourceGroup && destinationGroup) {
-        if (
-          sourceGroup.title === destinationGroup.title &&
-          destinationIndex &&
-          sourceIndex < destinationIndex
-        ) {
-          destinationIndex -= 1;
+      setUserDashboard((groups) => {
+        const sourceGroup = groups.find((group) => group.title === sourceTitle);
+        const destinationGroup = groups.find(
+          (group) => group.title === destinationTitle,
+        );
+        if (!sourceGroup || !destinationGroup) return groups;
+
+        const sameGroup = sourceGroup.title === destinationGroup.title;
+        // Copy into a local so the updater stays pure (React StrictMode may
+        // invoke it twice in development).
+        let destIndex = destinationIndex;
+        if (sameGroup && destIndex && sourceIndex < destIndex) {
+          destIndex -= 1;
         }
-        if (
-          sourceGroup.title === destinationGroup.title &&
-          (destinationIndex === null || sourceIndex === destinationIndex)
-        ) {
-          return;
+        if (sameGroup && (destIndex === null || sourceIndex === destIndex)) {
+          return groups; // Nothing to do
         }
 
-        if (sourceGroup.title === destinationGroup.title) {
+        if (sameGroup) {
           const sourceItems = [...sourceGroup.items];
           const [removed] = sourceItems.splice(sourceIndex, 1);
-          if (destinationIndex === null) {
-            destinationIndex = sourceItems.length;
+          if (destIndex === null) {
+            destIndex = sourceItems.length;
           }
-          sourceItems.splice(destinationIndex, 0, removed);
-
-          setUserDashboard((groups) =>
-            groups.map((group) =>
-              group.title === sourceGroup.title
-                ? { ...group, items: sourceItems }
-                : group,
-            ),
+          sourceItems.splice(destIndex, 0, removed);
+          return groups.map((group) =>
+            group.title === sourceGroup.title
+              ? { ...group, items: sourceItems }
+              : group,
           );
-        } else {
-          const sourceItems = [...sourceGroup.items];
-          const [removed] = sourceItems.splice(sourceIndex, 1);
-          const destinationItems = [...destinationGroup.items];
-          if (destinationIndex === null) {
-            destinationIndex = destinationItems.length;
-          }
-          destinationItems.splice(destinationIndex, 0, removed);
+        }
 
-          setUserDashboard((groups) =>
-            groups.map((group) =>
-              group.title === sourceGroup.title
-                ? { ...group, items: sourceItems }
-                : group.title === destinationGroup.title
-                  ? { ...group, items: destinationItems }
-                  : group,
-            ),
-          );
+        const sourceItems = [...sourceGroup.items];
+        const [removed] = sourceItems.splice(sourceIndex, 1);
+        const destinationItems = [...destinationGroup.items];
+        if (destIndex === null) {
+          destIndex = destinationItems.length;
         }
-      }
+        destinationItems.splice(destIndex, 0, removed);
+        return groups.map((group) =>
+          group.title === sourceGroup.title
+            ? { ...group, items: sourceItems }
+            : group.title === destinationGroup.title
+              ? { ...group, items: destinationItems }
+              : group,
+        );
+      });
     }
   }, [setUserDashboard]);
 }