narrow down & consolidate group selecting helper

src/actions/actionDuplicateSelection.tsx

@@ -263,8 +263,7 @@ const duplicateElements = (
       ...appState,
       ...selectGroupsForSelectedElements(
         {
-          ...appState,
+          editingGroupId: appState.editingGroupId,
-          selectedGroupIds: {},
           selectedElementIds: nextElementsToSelect.reduce(
             (acc: Record<ExcalidrawElement["id"], true>, element) => {
               if (!isBoundToContainer(element)) {

src/actions/actionGroup.tsx

@@ -215,7 +215,7 @@ export const actionUngroup = register({
     });
 
     const updateAppState = selectGroupsForSelectedElements(
-      { ...appState, selectedGroupIds: {} },
+      appState,
       getNonDeletedElements(nextElements),
       appState,
       null,

src/actions/actionSelectAll.ts

@@ -32,13 +32,6 @@ export const actionSelectAll = register({
         ...appState,
         ...selectGroupsForSelectedElements(
           {
-            ...appState,
-            selectedLinearElement:
-              // single linear element selected
-              Object.keys(selectedElementIds).length === 1 &&
-              isLinearElement(elements[0])
-                ? new LinearElementEditor(elements[0], app.scene)
-                : null,
             editingGroupId: null,
             selectedElementIds,
           },
@@ -46,6 +39,12 @@ export const actionSelectAll = register({
           appState,
           app,
         ),
+        selectedLinearElement:
+          // single linear element selected
+          Object.keys(selectedElementIds).length === 1 &&
+          isLinearElement(elements[0])
+            ? new LinearElementEditor(elements[0], app.scene)
+            : null,
       },
       commitToHistory: true,
     };

src/components/App.tsx

@@ -177,7 +177,6 @@ import {
   getSelectedGroupIds,
   isElementInGroup,
   isSelectedViaGroup,
-  selectGroups,
   selectGroupsForSelectedElements,
 } from "../groups";
 import History from "../history";
@@ -1706,7 +1705,7 @@ class App extends React.Component<AppProps, AppState> {
     this.library.destroy();
     clearTimeout(touchTimeout);
     isSomeElementSelected.clearCache();
-    selectGroups.clearCache();
+    selectGroupsForSelectedElements.clearCache();
     touchTimeout = 0;
   }
 
@@ -2300,7 +2299,6 @@ class App extends React.Component<AppProps, AppState> {
       excludeElementsInFramesFromSelection(newElements);
 
     this.setState(
-      selectGroupsForSelectedElements(
       {
         ...this.state,
         // keep sidebar (presumably the library) open if it's docked and
@@ -2314,6 +2312,9 @@ class App extends React.Component<AppProps, AppState> {
           jotaiStore.get(isSidebarDockedAtom)
             ? this.state.openSidebar
             : null,
+        ...selectGroupsForSelectedElements(
+          {
+            editingGroupId: null,
             selectedElementIds: nextElementsToSelect.reduce(
               (acc: Record<ExcalidrawElement["id"], true>, element) => {
                 if (!isBoundToContainer(element)) {
@@ -2323,12 +2324,12 @@ class App extends React.Component<AppProps, AppState> {
               },
               {},
             ),
-          selectedGroupIds: {},
           },
           this.scene.getNonDeletedElements(),
           this.state,
           this,
         ),
+      },
       () => {
         if (opts.files) {
           this.addNewImagesToImageCache();
@@ -3589,19 +3590,18 @@ class App extends React.Component<AppProps, AppState> {
         getSelectedGroupIdForElement(hitElement, this.state.selectedGroupIds);
 
       if (selectedGroupId) {
-        this.setState((prevState) =>
+        this.setState((prevState) => ({
-          selectGroupsForSelectedElements(
-            {
           ...prevState,
+          ...selectGroupsForSelectedElements(
+            {
               editingGroupId: selectedGroupId,
               selectedElementIds: { [hitElement!.id]: true },
-              selectedGroupIds: {},
             },
             this.scene.getNonDeletedElements(),
             prevState,
             this,
           ),
-        );
+        }));
         return;
       }
     }
@@ -5096,19 +5096,21 @@ class App extends React.Component<AppProps, AppState> {
                   }
                 }
 
-                return selectGroupsForSelectedElements(
+                return {
+                  ...selectGroupsForSelectedElements(
                     {
-                    ...prevState,
+                      editingGroupId: prevState.editingGroupId,
                       selectedElementIds: nextSelectedElementIds,
-                    showHyperlinkPopup:
-                      hitElement.link || isEmbeddableElement(hitElement)
-                        ? "info"
-                        : false,
                     },
                     this.scene.getNonDeletedElements(),
                     prevState,
                     this,
-                );
+                  ),
+                  showHyperlinkPopup:
+                    hitElement.link || isEmbeddableElement(hitElement)
+                      ? "info"
+                      : false,
+                };
               });
               pointerDownState.hit.wasAddedToSelection = true;
             }
@@ -6028,20 +6030,20 @@ class App extends React.Component<AppProps, AppState> {
               }
             }
 
-            return selectGroupsForSelectedElements(
+            prevState = !shouldReuseSelection
+              ? { ...prevState, selectedGroupIds: {}, editingGroupId: null }
+              : prevState;
+
+            return {
+              ...selectGroupsForSelectedElements(
                 {
-                ...prevState,
+                  editingGroupId: prevState.editingGroupId,
-                ...(!shouldReuseSelection && {
-                  selectedGroupIds: {},
-                  editingGroupId: null,
-                }),
                   selectedElementIds: nextSelectedElementIds,
-                showHyperlinkPopup:
+                },
-                  elementsWithinSelection.length === 1 &&
+                this.scene.getNonDeletedElements(),
-                  (elementsWithinSelection[0].link ||
+                prevState,
-                    isEmbeddableElement(elementsWithinSelection[0]))
+                this,
-                    ? "info"
+              ),
-                    : false,
               // select linear element only when we haven't box-selected anything else
               selectedLinearElement:
                 elementsWithinSelection.length === 1 &&
@@ -6051,11 +6053,13 @@ class App extends React.Component<AppProps, AppState> {
                       this.scene,
                     )
                   : null,
-              },
+              showHyperlinkPopup:
-              this.scene.getNonDeletedElements(),
+                elementsWithinSelection.length === 1 &&
-              prevState,
+                (elementsWithinSelection[0].link ||
-              this,
+                  isEmbeddableElement(elementsWithinSelection[0]))
-            );
+                  ? "info"
+                  : false,
+            };
           });
         }
       }
@@ -6656,10 +6660,16 @@ class App extends React.Component<AppProps, AppState> {
                   { selectedElementIds: newSelectedElementIds },
                 );
 
-                return selectGroupsForSelectedElements(
+                return {
+                  ...selectGroupsForSelectedElements(
                     {
-                    ...prevState,
+                      editingGroupId: prevState.editingGroupId,
                       selectedElementIds: newSelectedElementIds,
+                    },
+                    this.scene.getNonDeletedElements(),
+                    prevState,
+                    this,
+                  ),
                   // set selectedLinearElement only if thats the only element selected
                   selectedLinearElement:
                     newSelectedElements.length === 1 &&
@@ -6669,11 +6679,7 @@ class App extends React.Component<AppProps, AppState> {
                           this.scene,
                         )
                       : prevState.selectedLinearElement,
-                  },
+                };
-                  this.scene.getNonDeletedElements(),
-                  prevState,
-                  this,
-                );
               });
             }
           } else if (
@@ -6701,19 +6707,21 @@ class App extends React.Component<AppProps, AppState> {
                   delete nextSelectedElementIds[element.id];
                 });
 
-              return selectGroupsForSelectedElements(
+              return {
+                ...selectGroupsForSelectedElements(
                   {
-                  ...prevState,
+                    editingGroupId: prevState.editingGroupId,
                     selectedElementIds: nextSelectedElementIds,
-                  showHyperlinkPopup:
-                    hitElement.link || isEmbeddableElement(hitElement)
-                      ? "info"
-                      : false,
                   },
                   this.scene.getNonDeletedElements(),
                   prevState,
                   this,
-              );
+                ),
+                showHyperlinkPopup:
+                  hitElement.link || isEmbeddableElement(hitElement)
+                    ? "info"
+                    : false,
+              };
             });
           } else {
             // add element to selection while keeping prev elements selected
@@ -6731,8 +6739,13 @@ class App extends React.Component<AppProps, AppState> {
           this.setState((prevState) => ({
             ...selectGroupsForSelectedElements(
               {
-                ...prevState,
+                editingGroupId: prevState.editingGroupId,
                 selectedElementIds: { [hitElement.id]: true },
+              },
+              this.scene.getNonDeletedElements(),
+              prevState,
+              this,
+            ),
             selectedLinearElement:
               isLinearElement(hitElement) &&
               // Don't set `selectedLinearElement` if its same as the hitElement, this is mainly to prevent resetting the `hoverPointIndex` to -1.
@@ -6740,11 +6753,6 @@ class App extends React.Component<AppProps, AppState> {
               prevState.selectedLinearElement?.elementId !== hitElement.id
                 ? new LinearElementEditor(hitElement, this.scene)
                 : prevState.selectedLinearElement,
-              },
-              this.scene.getNonDeletedElements(),
-              prevState,
-              this,
-            ),
           }));
         }
       }
@@ -7595,16 +7603,16 @@ class App extends React.Component<AppProps, AppState> {
               ...this.state,
               ...selectGroupsForSelectedElements(
                 {
-                  ...this.state,
+                  editingGroupId: this.state.editingGroupId,
                   selectedElementIds: { [element.id]: true },
-                  selectedLinearElement: isLinearElement(element)
-                    ? new LinearElementEditor(element, this.scene)
-                    : null,
                 },
                 this.scene.getNonDeletedElements(),
                 this.state,
                 this,
               ),
+              selectedLinearElement: isLinearElement(element)
+                ? new LinearElementEditor(element, this.scene)
+                : null,
             }
           : this.state),
         showHyperlinkPopup: false,

src/groups.ts

@@ -55,25 +55,29 @@ export const selectGroup = (
   };
 };
 
-export const selectGroups = (function () {
+export const selectGroupsForSelectedElements = (function () {
+  type SelectGroupsReturnType = Pick<
+    InteractiveCanvasAppState,
+    "selectedGroupIds" | "editingGroupId" | "selectedElementIds"
+  >;
+
   let lastSelectedElements: readonly NonDeleted<ExcalidrawElement>[] | null =
     null;
   let lastElements: readonly NonDeleted<ExcalidrawElement>[] | null = null;
-  let lastAppState: InteractiveCanvasAppState | null = null;
+  let lastReturnValue: SelectGroupsReturnType | null = null;
 
-  const ret = (
+  const _selectGroups = (
     selectedElements: readonly NonDeleted<ExcalidrawElement>[],
     elements: readonly NonDeleted<ExcalidrawElement>[],
-    appState: InteractiveCanvasAppState,
+    appState: Pick<AppState, "selectedElementIds" | "editingGroupId">,
-  ): InteractiveCanvasAppState => {
+  ): SelectGroupsReturnType => {
     if (
-      lastAppState !== undefined &&
+      lastReturnValue !== undefined &&
       elements === lastElements &&
       selectedElements === lastSelectedElements &&
-      appState.editingGroupId === lastAppState?.editingGroupId &&
+      appState.editingGroupId === lastReturnValue?.editingGroupId
-      appState.selectedGroupIds === lastAppState?.selectedGroupIds
     ) {
-      return lastAppState;
+      return lastReturnValue;
     }
 
     const selectedGroupIds: Record<GroupId, boolean> = {};
@@ -126,8 +130,8 @@ export const selectGroups = (function () {
     lastElements = elements;
     lastSelectedElements = selectedElements;
 
-    lastAppState = {
+    lastReturnValue = {
-      ...appState,
+      editingGroupId: appState.editingGroupId,
       selectedGroupIds,
       selectedElementIds: {
         ...appState.selectedElementIds,
@@ -135,16 +139,55 @@ export const selectGroups = (function () {
       },
     };
 
-    return lastAppState;
+    return lastReturnValue;
   };
 
-  ret.clearCache = () => {
+  /**
+   * When you select an element, you often want to actually select the whole group it's in, unless
+   * you're currently editing that group.
+   */
+  const selectGroupsForSelectedElements = (
+    appState: Pick<AppState, "selectedElementIds" | "editingGroupId">,
+    elements: readonly NonDeletedExcalidrawElement[],
+    prevAppState: InteractiveCanvasAppState,
+    /**
+     * supply null in cases where you don't have access to App instance and
+     * you don't care about optimizing selectElements retrieval
+     */
+    app: AppClassProperties | null,
+  ): Pick<
+    InteractiveCanvasAppState,
+    "selectedGroupIds" | "editingGroupId" | "selectedElementIds"
+  > => {
+    const selectedElements = app
+      ? app.scene.getSelectedElements({
+          selectedElementIds: appState.selectedElementIds,
+          // supplying elements explicitly in case we're passed non-state elements
+          elements,
+        })
+      : getSelectedElements(elements, appState);
+
+    if (!selectedElements.length) {
+      return {
+        selectedGroupIds: {},
+        editingGroupId: null,
+        selectedElementIds: makeNextSelectedElementIds(
+          appState.selectedElementIds,
+          prevAppState,
+        ),
+      };
+    }
+
+    return _selectGroups(selectedElements, elements, appState);
+  };
+
+  selectGroupsForSelectedElements.clearCache = () => {
     lastElements = null;
     lastSelectedElements = null;
-    lastAppState = null;
+    lastReturnValue = null;
   };
 
-  return ret;
+  return selectGroupsForSelectedElements;
 })();
 
 /**
@@ -171,49 +214,6 @@ export const getSelectedGroupIds = (
     .filter(([groupId, isSelected]) => isSelected)
     .map(([groupId, isSelected]) => groupId);
 
-/**
- * When you select an element, you often want to actually select the whole group it's in, unless
- * you're currently editing that group.
- */
-export const selectGroupsForSelectedElements = (
-  appState: InteractiveCanvasAppState,
-  elements: readonly NonDeletedExcalidrawElement[],
-  prevAppState: InteractiveCanvasAppState,
-  /**
-   * supply null in cases where you don't have access to App instance and
-   * you don't care about optimizing selectElements retrieval
-   */
-  app: AppClassProperties | null,
-): InteractiveCanvasAppState => {
-  let nextAppState: InteractiveCanvasAppState = {
-    ...appState,
-    selectedGroupIds: {},
-  };
-
-  const selectedElements = app
-    ? app.scene.getSelectedElements({
-        selectedElementIds: appState.selectedElementIds,
-        // supplying elements explicitly in case we're passed non-state elements
-        elements,
-      })
-    : getSelectedElements(elements, appState);
-
-  if (!selectedElements.length) {
-    return {
-      ...nextAppState,
-      editingGroupId: null,
-      selectedElementIds: makeNextSelectedElementIds(
-        nextAppState.selectedElementIds,
-        prevAppState,
-      ),
-    };
-  }
-
-  nextAppState = selectGroups(selectedElements, elements, appState);
-
-  return nextAppState;
-};
-
 // given a list of elements, return the the actual group ids that should be selected
 // or used to update the elements
 export const selectGroupsFromGivenElements = (