Fix multi-selection interaction bugs in the Objects panel (follow-up to #8959) (#9002)

Don't show in changelog

Author: 4ian

newIDE/app/src/ObjectsList/EnumerateObjectFolderOrObject.js

@@ -166,9 +166,10 @@ const itemMatchesSearchText = (
 
 /**
  * Same case-insensitive substring match as TreeView's search filter.
- * Folders that only appear as ancestors of a match, or that still contain a
- * non-matching descendant, are not selectable: bulk ops would otherwise act
- * on hidden children.
+ * Used by "Select all" while searching: folders that only appear as ancestors
+ * of a match, or that still contain a non-matching descendant, are skipped so
+ * that this implicit selection cannot act on hidden children. Explicitly
+ * clicked rows are never filtered: a visible row is always selectable.
  */
 export const isSelectableWhileSearching = (
   objectFolderOrObject: gdObjectFolderOrObject,
@@ -203,25 +204,28 @@ export const enumerateAllChildrenInFolderMatchingSearch = (
 };
 
 /**
- * After a Ctrl+click that removes a folder from the selection, also drop
+ * After a Ctrl+click that explicitly deselects a folder, also drop its
  * descendants that Select All (or a previous range) had added. Otherwise
  * nested objects stay selected inside a collapsed folder.
+ *
+ * `removedObjectFolderOrObjects` must be the items explicitly toggled off by
+ * the gesture (as reported by TreeView), NOT the difference between the
+ * previous and next selections: a plain click on the child of a selected
+ * folder also "removes" the folder from the selection, but the clicked child
+ * must obviously stay selected.
  */
 export const dropDescendantsOfRemovedFolders = (
-  previous: Array<ObjectFolderOrObjectWithContext>,
+  removedObjectFolderOrObjects: Array<gdObjectFolderOrObject>,
   next: Array<ObjectFolderOrObjectWithContext>
 ): Array<ObjectFolderOrObjectWithContext> => {
-  const nextPtrs = new Set(next.map(item => item.objectFolderOrObject.ptr));
-  const removedFolders = previous.filter(
-    item =>
-      item.objectFolderOrObject.isFolder() &&
-      !nextPtrs.has(item.objectFolderOrObject.ptr)
+  const removedFolders = removedObjectFolderOrObjects.filter(
+    objectFolderOrObject => objectFolderOrObject.isFolder()
   );
   if (removedFolders.length === 0) return next;
   return next.filter(
     item =>
       !removedFolders.some(folder =>
-        item.objectFolderOrObject.isADescendantOf(folder.objectFolderOrObject)
+        item.objectFolderOrObject.isADescendantOf(folder)
       )
   );
 };

newIDE/app/src/ObjectsList/ObjectFolderOrObjectsClipboard.js

@@ -14,6 +14,8 @@ import {
 import { t } from '@lingui/macro';
 import { type I18n as I18nType } from '@lingui/core';
 
+const gd: libGDevelop = global.gd;
+
 export const OBJECT_FOLDER_OR_OBJECTS_CLIPBOARD_KIND = 'ObjectFolderOrObjects';
 
 // Kept for backward compatibility: read (but never written anymore) so that
@@ -317,3 +319,70 @@ export const pasteObjectFolderOrObjectsFromClipboard = ({
 
   return { createdObjects, topLevelObjectFolderOrObjects };
 };
+
+/**
+ * Paste the content of the clipboard at the given position and run the whole
+ * notification sequence shared by every paste entry point of the Objects
+ * panel: mark the project as modified, fire the object hooks (only when
+ * actual objects were created - pasting empty folders produces top-level
+ * items but no objects), and select the pasted items.
+ * Returns true when something was pasted.
+ */
+export const pasteObjectFolderOrObjectsAndNotify = ({
+  project,
+  globalObjectsContainer,
+  objectsContainer,
+  global,
+  destinationFolder,
+  positionInFolder,
+  onObjectModified,
+  onObjectPasted,
+  onObjectCreated,
+  selectObjectFolderOrObjectsWithContext,
+}: {|
+  project: gdProject,
+  globalObjectsContainer: gdObjectsContainer | null,
+  objectsContainer: gdObjectsContainer,
+  global: boolean,
+  destinationFolder: gdObjectFolderOrObject,
+  positionInFolder: number,
+  onObjectModified: (shouldForceUpdateList: boolean) => void,
+  onObjectPasted: ?(object: gdObject) => void,
+  onObjectCreated: (
+    objects: Array<gdObject>,
+    isTheFirstOfItsTypeInProject: boolean
+  ) => void,
+  selectObjectFolderOrObjectsWithContext: (
+    items: Array<ObjectFolderOrObjectWithContext>
+  ) => void,
+|}): boolean => {
+  const isTheFirstOfItsTypeInProject = getObjectFolderOrObjectsClipboardObjectTypes().some(
+    objectType => !gd.UsedObjectTypeFinder.scanProject(project, objectType)
+  );
+
+  const pastedContent = pasteObjectFolderOrObjectsFromClipboard({
+    project,
+    globalObjectsContainer,
+    objectsContainer,
+    global,
+    destinationFolder,
+    positionInFolder,
+  });
+  if (!pastedContent) return false;
+  const { createdObjects, topLevelObjectFolderOrObjects } = pastedContent;
+  if (topLevelObjectFolderOrObjects.length === 0) return false;
+
+  // onObjectModified(true) already calls forceUpdateList internally.
+  onObjectModified(true);
+  if (createdObjects.length > 0) {
+    if (onObjectPasted) onObjectPasted(createdObjects[0]);
+    onObjectCreated(createdObjects, isTheFirstOfItsTypeInProject);
+  }
+  selectObjectFolderOrObjectsWithContext(
+    topLevelObjectFolderOrObjects.map(objectFolderOrObject => ({
+      objectFolderOrObject,
+      global,
+    }))
+  );
+  return true;
+};

newIDE/app/src/ObjectsList/ObjectFolderOrObjectsClipboard.spec.js

@@ -175,20 +175,30 @@ describe('ObjectFolderOrObjectsClipboard', () => {
     const objectInFolder = subFolder.getChildAt(0);
     const sibling = rootFolder.getChildAt(0);
 
-    const previous = [
-      { global: false, objectFolderOrObject: subFolder },
-      { global: false, objectFolderOrObject: objectInFolder },
-      { global: false, objectFolderOrObject: sibling },
-    ];
     const nextAfterDeselectingFolder = [
       { global: false, objectFolderOrObject: objectInFolder },
       { global: false, objectFolderOrObject: sibling },
     ];
 
+    // Ctrl+click deselected the folder: its still-selected child is dropped.
     expect(
-      dropDescendantsOfRemovedFolders(previous, nextAfterDeselectingFolder)
+      dropDescendantsOfRemovedFolders([subFolder], nextAfterDeselectingFolder)
     ).toEqual([{ global: false, objectFolderOrObject: sibling }]);
 
+    // A plain click on the child of a selected folder is not a toggle-off
+    // gesture (no removed items): the clicked child stays selected.
+    expect(
+      dropDescendantsOfRemovedFolders(
+        [],
+        [{ global: false, objectFolderOrObject: objectInFolder }]
+      )
+    ).toEqual([{ global: false, objectFolderOrObject: objectInFolder }]);
+
+    // Deselecting a plain object never drops anything.
+    expect(
+      dropDescendantsOfRemovedFolders([sibling], nextAfterDeselectingFolder)
+    ).toEqual(nextAfterDeselectingFolder);
+
     project.delete();
   });
 

newIDE/app/src/ObjectsList/ObjectFolderTreeViewItemContent.js

@@ -16,8 +16,7 @@ import {
   writeObjectFolderOrObjectsToClipboard,
   hasObjectFolderOrObjectsInClipboard,
   getObjectFolderOrObjectsClipboardSummaryName,
-  getObjectFolderOrObjectsClipboardObjectTypes,
-  pasteObjectFolderOrObjectsFromClipboard,
+  pasteObjectFolderOrObjectsAndNotify,
 } from './ObjectFolderOrObjectsClipboard';
 import { duplicateObjectFolderOrObjects } from './ObjectFolderOrObjectsDuplicate';
 import { renderQuickCustomizationMenuItems } from '../QuickCustomization/QuickCustomizationMenuItems';
@@ -458,37 +457,22 @@ export class ObjectFolderTreeViewItemContent implements TreeViewItemContent {
       selectObjectFolderOrObjectsWithContext,
     } = this.props;
 
-    const isTheFirstOfItsTypeInProject = getObjectFolderOrObjectsClipboardObjectTypes().some(
-      objectType => !gd.UsedObjectTypeFinder.scanProject(project, objectType)
-    );
-
-    const pastedContent = pasteObjectFolderOrObjectsFromClipboard({
+    const pasted = pasteObjectFolderOrObjectsAndNotify({
       project,
       globalObjectsContainer,
       objectsContainer,
       global: this._isGlobal,
       destinationFolder: this.objectFolder,
       positionInFolder: this.objectFolder.getChildrenCount(),
+      onObjectModified,
+      onObjectPasted,
+      onObjectCreated,
+      selectObjectFolderOrObjectsWithContext,
     });
-    if (!pastedContent) return;
-    const { createdObjects, topLevelObjectFolderOrObjects } = pastedContent;
-    if (topLevelObjectFolderOrObjects.length === 0) return;
-
-    if (createdObjects.length > 0) {
-      onObjectCreated(createdObjects, isTheFirstOfItsTypeInProject);
-    }
-    onObjectModified(true);
-    if (onObjectPasted && createdObjects.length > 0)
-      onObjectPasted(createdObjects[0]);
+    if (!pasted) return;
     expandFolders([
       { objectFolderOrObject: this.objectFolder, global: this._isGlobal },
     ]);
-    selectObjectFolderOrObjectsWithContext(
-      topLevelObjectFolderOrObjects.map(pastedObjectFolderOrObject => ({
-        objectFolderOrObject: pastedObjectFolderOrObject,
-        global: this._isGlobal,
-      }))
-    );
   }
 
   duplicate(): void {

newIDE/app/src/ObjectsList/ObjectTreeViewItemContent.js

@@ -21,8 +21,7 @@ import {
   writeObjectFolderOrObjectsToClipboard,
   hasObjectFolderOrObjectsInClipboard,
   getObjectFolderOrObjectsClipboardSummaryName,
-  getObjectFolderOrObjectsClipboardObjectTypes,
-  pasteObjectFolderOrObjectsFromClipboard,
+  pasteObjectFolderOrObjectsAndNotify,
 } from './ObjectFolderOrObjectsClipboard';
 import { type ObjectEditorTab } from '../ObjectEditor/ObjectEditorDialog';
 import type { ObjectWithContext } from '../ObjectsList/EnumerateObjects';
@@ -293,9 +292,11 @@ export class ObjectTreeViewItemContent implements TreeViewItemContent {
 
   onClick(): void {
     // Selection itself is entirely handled by TreeView (single click, Ctrl/Cmd
-    // toggle, Shift range) through `onSelectItems`. Do not select here, as it
-    // would override that selection (in particular, it would collapse any
-    // multi-selection back to this single item).
+    // toggle, Shift range) through `onSelectItems` - including a plain click
+    // on the already-selected row, which is re-notified so the parent can
+    // bring the selection back to the front of the properties panel. Do not
+    // select here, as it would override that selection (in particular, it
+    // would collapse any multi-selection back to this single item).
   }
 
   rename(newName: string): void {
@@ -647,45 +648,32 @@ export class ObjectTreeViewItemContent implements TreeViewItemContent {
     const objectFolderOrObject = this._getAliveObjectFolderOrObject();
     if (!objectFolderOrObject) return;
 
-    const { project, globalObjectsContainer, objectsContainer } = this.props;
     const parentFolder = exceptionallyGuardAgainstDeadObject(
       objectFolderOrObject.getParent()
     );
     if (!parentFolder) return;
 
-    const isTheFirstOfItsTypeInProject = getObjectFolderOrObjectsClipboardObjectTypes().some(
-      objectType => !gd.UsedObjectTypeFinder.scanProject(project, objectType)
-    );
-
-    const pastedContent = pasteObjectFolderOrObjectsFromClipboard({
+    const {
+      project,
+      globalObjectsContainer,
+      objectsContainer,
+      onObjectPasted,
+      onObjectModified,
+      onObjectCreated,
+      selectObjectFolderOrObjectsWithContext,
+    } = this.props;
+    pasteObjectFolderOrObjectsAndNotify({
       project,
       globalObjectsContainer,
       objectsContainer,
       global: this._isGlobal,
       destinationFolder: parentFolder,
       positionInFolder: parentFolder.getChildPosition(objectFolderOrObject) + 1,
-    });
-    if (!pastedContent) return;
-    const { createdObjects, topLevelObjectFolderOrObjects } = pastedContent;
-    if (topLevelObjectFolderOrObjects.length === 0) return;
-
-    const {
-      onObjectPasted,
       onObjectModified,
+      onObjectPasted,
       onObjectCreated,
       selectObjectFolderOrObjectsWithContext,
-    } = this.props;
-    onObjectModified(true);
-    if (createdObjects.length > 0) {
-      if (onObjectPasted) onObjectPasted(createdObjects[0]);
-      onObjectCreated(createdObjects, isTheFirstOfItsTypeInProject);
-    }
-    selectObjectFolderOrObjectsWithContext(
-      topLevelObjectFolderOrObjects.map(pastedObjectFolderOrObject => ({
-        objectFolderOrObject: pastedObjectFolderOrObject,
-        global: this._isGlobal,
-      }))
-    );
+    });
   }
 
   duplicate(): void {

newIDE/app/src/ObjectsList/UseBulkObjectOperations.js

@@ -16,8 +16,7 @@ import {
   serializeObjectFolderOrObjectsForClipboard,
   writeObjectFolderOrObjectsToClipboard,
   hasObjectFolderOrObjectsInClipboard,
-  getObjectFolderOrObjectsClipboardObjectTypes,
-  pasteObjectFolderOrObjectsFromClipboard,
+  pasteObjectFolderOrObjectsAndNotify,
   getPasteMenuLabel,
   getUniqueFolderName,
 } from './ObjectFolderOrObjectsClipboard';
@@ -250,36 +249,18 @@ function useBulkObjectOperations({
         ? objectFolderOrObject.getChildrenCount()
         : destinationFolder.getChildPosition(objectFolderOrObject) + 1;
 
-      const isTheFirstOfItsTypeInProject = getObjectFolderOrObjectsClipboardObjectTypes().some(
-        objectType => !gd.UsedObjectTypeFinder.scanProject(project, objectType)
-      );
-
-      const pastedContent = pasteObjectFolderOrObjectsFromClipboard({
+      pasteObjectFolderOrObjectsAndNotify({
         project,
         globalObjectsContainer,
         objectsContainer,
         global,
         destinationFolder,
         positionInFolder,
+        onObjectModified,
+        onObjectPasted,
+        onObjectCreated,
+        selectObjectFolderOrObjectsWithContext,
       });
-      if (!pastedContent) return;
-      const { createdObjects, topLevelObjectFolderOrObjects } = pastedContent;
-      if (topLevelObjectFolderOrObjects.length === 0) return;
-
-      // onObjectModified(true) already calls forceUpdateList internally.
-      onObjectModified(true);
-      // Only fire object hooks when actual objects were created; pasting
-      // empty folders produces topLevelObjectFolderOrObjects but no objects.
-      if (createdObjects.length > 0) {
-        if (onObjectPasted) onObjectPasted(createdObjects[0]);
-        onObjectCreated(createdObjects, isTheFirstOfItsTypeInProject);
-      }
-      selectObjectFolderOrObjectsWithContext(
-        topLevelObjectFolderOrObjects.map(pastedObjectFolderOrObject => ({
-          objectFolderOrObject: pastedObjectFolderOrObject,
-          global,
-        }))
-      );
     },
     [
       isListLocked,
@@ -306,25 +287,31 @@ function useBulkObjectOperations({
       );
       if (candidates.length === 0) return;
 
-      // Filter out items that cannot be promoted, showing a warning per
-      // item rather than aborting the whole operation.
+      // Filter out items that cannot be promoted, then show a single warning
+      // listing all the name conflicts rather than one blocking dialog each.
+      const conflictingObjectNames = [];
       const objectItems = candidates.filter(item => {
         const objectName = item.objectFolderOrObject.getObject().getName();
         if (!objectsContainer.hasObjectNamed(objectName)) return false;
         if (globalObjectsContainer.hasObjectNamed(objectName)) {
-          showWarningBox(
-            i18n._(
-              t`A global object with this name already exists. Please change the object name before setting it as a global object`
-            ),
-            { delayToNextTick: true }
-          );
+          conflictingObjectNames.push(objectName);
           return false;
         }
         if (beforeSetAsGlobalObject && !beforeSetAsGlobalObject(objectName)) {
           return false;
         }
         return true;
       });
+      if (conflictingObjectNames.length > 0) {
+        showWarningBox(
+          i18n._(
+            t`Global objects with these names already exist: ${conflictingObjectNames.join(
+              ', '
+            )}. Please rename the objects before setting them as global objects.`
+          ),
+          { delayToNextTick: true }
+        );
+      }
       if (objectItems.length === 0) return;
 
       const answer = Window.showConfirmDialog(