fix: pane relayout order

js/main.js

@@ -14,7 +14,13 @@
 import 'fetch';
 import 'rc-tree-select/assets/index.less';
 
-import React, { useContext, useEffect, useRef, useState } from 'react';
+import React, {
+  useContext,
+  useEffect,
+  useLayoutEffect,
+  useRef,
+  useState,
+} from 'react';
 import ReactDOM from 'react-dom';
 import ReactGridLayout, {
   getLayoutItem,
@@ -299,7 +305,7 @@ const App = () => {
         showSavedStateRecoveryToast();
       });
       if (_bin.current == null) {
-        rebin();
+        _bin.current = createBin(newLayout, windowSize.current.cols);
       }
       let paneLayout;
       if (stored) {
@@ -432,7 +438,7 @@ const App = () => {
         return { ...prev, panes: newPanes, layout: newLayout };
       });
       setFocusedPaneID(focusedPaneID === paneID ? null : focusedPaneID);
-      callbacks.current.push('relayout');
+      relayout();
     }
   };
 
@@ -443,7 +449,6 @@ const App = () => {
     Object.keys(storeData.panes).map((paneID) => {
       closePane(paneID, false, false);
     });
-    rebin();
     setStoreData((prev) => ({
       ...prev,
       layout: [],
@@ -617,32 +622,35 @@ const App = () => {
     updateLayout(layout);
   };
 
-  const rebin = (layout) => {
-    layout = layout ? layout : storeData.layout;
-    let layoutID = selection.layoutID;
+  const applySavedLayout = (layout, layoutID, layoutMap) => {
     if (layoutID !== DEFAULT_LAYOUT) {
-      let envLayoutList = getCurrLayoutList();
-      let layoutMap = envLayoutList.get(selection.layoutID);
-      layout = layout.map((paneLayout) => {
+      return layout.map((paneLayout) => {
         if (layoutMap.has(paneLayout.i)) {
           let storedVals = layoutMap.get(paneLayout.i);
-          paneLayout.h = storedVals[1];
-          paneLayout.height = storedVals[1];
-          paneLayout.w = storedVals[2];
-          paneLayout.width = storedVals[2];
+          return {
+            ...paneLayout,
+            h: storedVals[1],
+            height: storedVals[1],
+            w: storedVals[2],
+            width: storedVals[2],
+          };
         }
         return paneLayout;
       });
     }
+
+    return layout;
+  };
+
+  const createBin = (layout, cols) => {
     let contents = layout.map((paneLayout) => {
       return {
         width: paneLayout.w,
         height: paneLayout.h,
       };
     });
 
-    _bin.current = new Bin.ShelfFirst(contents, windowSize.current.cols);
-    return layout;
+    return new Bin.ShelfFirst(contents, cols);
   };
 
   const getCurrLayoutList = () => {
@@ -653,59 +661,72 @@ const App = () => {
     }
   };
 
-  const relayout = () => {
-    let layout = rebin();
-
-    let sorted = sortLayout(layout);
-    let newPanes = Object.assign({}, storeData.panes);
-    let filter = getValidFilter(filterString);
-    let old_sorted = sorted.slice();
-    let layoutID = selection.layoutID;
+  const relayout = ({
+    layoutID = selection.layoutID,
+    filterString: nextFilterString = filterString,
+  } = {}) => {
     let envLayoutList = getCurrLayoutList();
-    let layoutMap = envLayoutList.get(selection.layoutID);
-    // Sort out things that were filtered away
-    sorted = sorted.sort(function (a, b) {
-      let diff =
-        (newPanes[a.i].title.match(filter) != null) -
-        (newPanes[b.i].title.match(filter) != null);
-      if (diff != 0) {
-        return -diff;
-      } else if (layoutID !== DEFAULT_LAYOUT) {
-        let aVal = layoutMap.has(a.i) ? -layoutMap.get(a.i)[0] : 1;
-        let bVal = layoutMap.has(b.i) ? -layoutMap.get(b.i)[0] : 1;
-        let diff = bVal - aVal;
+    let filter = getValidFilter(nextFilterString);
+    let cols = windowSize.current.cols;
+
+    setStoreData((prev) => {
+      let layoutMap = envLayoutList.get(layoutID);
+      let sorted = sortLayout(prev.layout);
+      let old_sorted = sorted.slice();
+      let newPanes = Object.assign({}, prev.panes);
+
+      // Sort out things that were filtered away
+      sorted = sorted.sort(function (a, b) {
+        let diff =
+          (newPanes[a.i].title.match(filter) != null) -
+          (newPanes[b.i].title.match(filter) != null);
         if (diff != 0) {
-          // At least one of the two was in the layout map.
-          return diff;
+          return -diff;
+        } else if (layoutID !== DEFAULT_LAYOUT) {
+          let aVal = layoutMap.has(a.i) ? -layoutMap.get(a.i)[0] : 1;
+          let bVal = layoutMap.has(b.i) ? -layoutMap.get(b.i)[0] : 1;
+          let diff = bVal - aVal;
+          if (diff != 0) {
+            // At least one of the two was in the layout map.
+            return diff;
+          }
         }
-      }
-      return old_sorted.indexOf(a) - old_sorted.indexOf(b); // stable sort
-    });
+        return old_sorted.indexOf(a) - old_sorted.indexOf(b); // stable sort
+      });
 
-    let newLayout = sorted.map((paneLayout, idx) => {
-      let pos = _bin.current.position(idx, windowSize.current.cols);
+      // The bin packer indexes its dimensions by pane order, so initialize it
+      // only after the final filtered/saved-view order has been determined.
+      sorted = applySavedLayout(sorted, layoutID, layoutMap);
+      let bin = createBin(sorted, cols);
 
-      newPanes[paneLayout.i].i = idx;
+      let newLayout = sorted.map((paneLayout, idx) => {
+        let pos = bin.position(idx, cols);
 
-      return Object.assign({}, paneLayout, pos);
-    });
+        newPanes[paneLayout.i] = {
+          ...newPanes[paneLayout.i],
+          i: idx,
+        };
 
-    setStoreData((prev) => ({
-      ...prev,
-      panes: newPanes,
-    }));
-    updateLayout(newLayout);
+        return Object.assign({}, paneLayout, pos);
+      });
+
+      return {
+        ...prev,
+        panes: newPanes,
+        layout: newLayout,
+      };
+    });
   };
 
   const updateLayout = (layout) => {
     setStoreData((prev) => ({ ...prev, layout: layout }));
-    // TODO this is very non-conventional react, someday it shall be fixed but
-    // for now it's important to fix relayout grossness
-    storeData.layout = layout;
   };
   const resizePaneLive = (layout) => {
     updateLayout(layout);
   };
+  useLayoutEffect(() => {
+    _bin.current = createBin(storeData.layout, windowSize.current.cols);
+  }, [storeData.layout]);
   useEffect(() => {
     clearTimeout(localStorageTimer.current);
     localStorageTimer.current = setTimeout(() => {
@@ -724,13 +745,8 @@ const App = () => {
       ...prev,
       layoutID: newLayoutID,
     }));
-    // TODO this is very non-conventional react, someday it shall be fixed but
-    // for now it's important to fix relayout grossness
-    selection.layoutID = newLayoutID;
-    if (selection.layoutID !== DEFAULT_LAYOUT) {
-      callbacks.current.push('relayout');
-      callbacks.current.push('relayout');
-      callbacks.current.push('relayout');
+    if (newLayoutID !== DEFAULT_LAYOUT) {
+      relayout({ layoutID: newLayoutID });
     }
   };
 
@@ -827,13 +843,13 @@ const App = () => {
     }
   }, []);
 
-  // flush pre-render callbacks
+  // Run callbacks after state updates have been committed.
   const callbacks = useRef([]);
-  callbacks.current.forEach((cb) => {
-    if (cb === 'relayout') relayout();
-    else if (cb) cb();
+  useEffect(() => {
+    let pendingCallbacks = callbacks.current;
+    callbacks.current = [];
+    pendingCallbacks.forEach((cb) => cb());
   });
-  callbacks.current = [];
 
   // ask server for envs after registration succeeded
   useEffect(() => {
@@ -1054,7 +1070,6 @@ const App = () => {
       layoutList={getCurrLayoutList()}
       onRepackButton={() => {
         relayout();
-        relayout();
       }}
       onViewChange={updateToLayout}
       onViewManageButton={() => setShowViewModal(!showViewModal)}
@@ -1071,12 +1086,13 @@ const App = () => {
     <FilterControls
       filter={filterString}
       onFilterChange={(ev) => {
-        setFilterString(ev.target.value);
-        callbacks.current.push('relayout');
+        let nextFilterString = ev.target.value;
+        setFilterString(nextFilterString);
+        relayout({ filterString: nextFilterString });
       }}
       onFilterClear={() => {
         setFilterString('');
-        callbacks.current.push('relayout');
+        relayout({ filterString: '' });
       }}
     />
   );