Avoid creating a getSnapshot copy to avoid track re-rendering on clic…

…k-and-drag
GMOD · Apr 12, 2024 · ed55e53 · ed55e53
1 parent 9027327
commit ed55e53
Show file tree

Hide file tree

Showing 2 changed files with 45 additions and 43 deletions.
diff --git a/packages/app-core/src/ui/App/ViewMenu.tsx b/packages/app-core/src/ui/App/ViewMenu.tsx
@@ -43,46 +43,12 @@ const ViewMenu = observer(function ({
     variant: 'popover',
   })
 
-  const items = [
-    ...(session.views.length > 1
-      ? [
-          {
-            label: 'Options',
-            type: 'subMenu',
-            subMenu: [
-              {
-                label: 'Move view to top',
-                icon: KeyboardDoubleArrowUpIcon,
-                onClick: () => session.moveViewToTop(model.id),
-              },
-              {
-                label: 'Move view up',
-                icon: KeyboardArrowUpIcon,
-                onClick: () => session.moveViewUp(model.id),
-              },
-              {
-                label: 'Move view down',
-                icon: KeyboardArrowDownIcon,
-                onClick: () => session.moveViewDown(model.id),
-              },
-              {
-                label: 'Move view to bottom',
-                icon: KeyboardDoubleArrowDownIcon,
-                onClick: () => session.moveViewToBottom(model.id),
-              },
-            ],
-          },
-        ]
-      : []),
-
-    // <=1.3.3 didn't use a function, so check as value also
-    ...((typeof menuItems === 'function' ? menuItems() : menuItems) || []),
-  ]
-
-  // note: This does not use CascadingMenuButton on purpose, because there was a confusing bug related to it!
-  // see https://github.com/GMOD/jbrowse-components/issues/4115
+  // note: This does not use CascadingMenuButton on purpose, because there was
+  // a confusing bug related to it! see
+  // https://github.com/GMOD/jbrowse-components/issues/4115
   //
-  // Make sure to test the Breakpoint split view menu checkboxes if you intend to change this
+  // Make sure to test the Breakpoint split view menu checkboxes if you
+  // intend to change this
   return (
     <>
       <IconButton
@@ -95,7 +61,42 @@ const ViewMenu = observer(function ({
       <CascadingMenu
         {...bindPopover(popupState)}
         onMenuItemClick={(_event: unknown, callback: () => void) => callback()}
-        menuItems={items}
+        menuItems={[
+          ...(session.views.length > 1
+            ? [
+                {
+                  label: 'Options',
+                  type: 'subMenu' as const,
+                  subMenu: [
+                    {
+                      label: 'Move view to top',
+                      icon: KeyboardDoubleArrowUpIcon,
+                      onClick: () => session.moveViewToTop(model.id),
+                    },
+                    {
+                      label: 'Move view up',
+                      icon: KeyboardArrowUpIcon,
+                      onClick: () => session.moveViewUp(model.id),
+                    },
+                    {
+                      label: 'Move view down',
+                      icon: KeyboardArrowDownIcon,
+                      onClick: () => session.moveViewDown(model.id),
+                    },
+                    {
+                      label: 'Move view to bottom',
+                      icon: KeyboardDoubleArrowDownIcon,
+                      onClick: () => session.moveViewToBottom(model.id),
+                    },
+                  ],
+                },
+              ]
+            : []),
+
+          // <=1.3.3 didn't use a function, so check as value also
+          ...((typeof menuItems === 'function' ? menuItems() : menuItems) ||
+            []),
+        ]}
         popupState={popupState}
       />
     </>

diff --git a/plugins/linear-genome-view/src/LinearGenomeView/model.ts b/plugins/linear-genome-view/src/LinearGenomeView/model.ts
@@ -808,9 +808,10 @@ export function stateModelFactory(pluginManager: PluginManager) {
         if (newIndex === -1) {
           throw new Error(`Track ID ${targetId} not found`)
         }
-        const track = getSnapshot(self.tracks[oldIndex])
-        self.tracks.splice(oldIndex, 1)
-        self.tracks.splice(newIndex, 0, track)
+
+        const tracks = self.tracks.filter((_, idx) => idx !== oldIndex)
+        tracks.splice(newIndex, 0, self.tracks[oldIndex])
+        self.tracks = cast(tracks)
       },
 
       /**