MenuBase: refactor _updateSelectedItems method (DevExpress#30739) (DevExpress#30747)

Author: juleari

packages/devextreme/js/__internal/ui/context_menu/menu_base.ts

@@ -728,7 +728,6 @@ class MenuBase<
             this._toggleItemSelection(selectedNode, false);
           }
           this._toggleItemSelection(node, true);
-          this._updateSelectedItems();
         }
         break;
       }
@@ -770,24 +769,26 @@ class MenuBase<
     return result;
   }
 
-  _updateSelectedItems(
-    oldSelection?: Item,
-    newSelection?: Item | null,
+  _updateSelectedItems(): void {}
+
+  _updateSelectedItem(
+    addedItem?: Item | null,
+    removedItem?: Item,
   ): void {
-    if (oldSelection || newSelection) {
-      this._fireSelectionChangeEvent(newSelection, oldSelection);
+    if (addedItem || removedItem) {
+      this._fireSelectionChangeEvent(addedItem, removedItem);
     }
   }
 
   _fireSelectionChangeEvent(
-    addedSelection: Item | null | undefined,
-    removedSelection: Item | undefined,
+    addedItem?: Item | null,
+    removedItem?: Item,
   ): void {
     this._createActionByOption('onSelectionChanged', {
       excludeValidators: ['disabled', 'readOnly'],
     })({
-      addedItems: [addedSelection],
-      removedItems: [removedSelection],
+      addedItems: [addedItem],
+      removedItems: [removedItem],
     });
   }
 
@@ -804,7 +805,7 @@ class MenuBase<
         this._toggleItemSelection(selectedNode, false);
       }
       this._toggleItemSelection(node, true);
-      this._updateSelectedItems(selectedItem, itemData);
+      this._updateSelectedItem(itemData, selectedItem);
       this._setOptionWithoutOptionChange('selectedItem', itemData);
     }
   }
@@ -816,7 +817,7 @@ class MenuBase<
 
     if (node?.internalFields.selected) {
       this._toggleItemSelection(node, false);
-      this._updateSelectedItems(selectedItem, null);
+      this._updateSelectedItem(null, selectedItem);
       this._setOptionWithoutOptionChange('selectedItem', null);
     }
   }

packages/devextreme/testing/tests/DevExpress.ui.widgets/menuBase.tests.js

@@ -1031,6 +1031,62 @@ QUnit.module('Selection', () => {
         $item.trigger('dxclick');
     });
 
+    QUnit.test('onSelectionChanged should have been called with correct arguments', function(assert) {
+        const items = [
+            { text: 'item1' },
+            { text: 'item2' },
+        ];
+        const selectionChangedHandler = sinon.stub();
+        const menuBase = createMenu({
+            items: items,
+            selectionMode: 'single',
+            selectByClick: true,
+            onSelectionChanged: selectionChangedHandler,
+        });
+        const $items = menuBase.element.find(`.${DX_MENU_ITEM_CLASS}`);
+        const $item1 = $items.eq(0);
+        const $item2 = $items.eq(1);
+
+        $item2.trigger('dxclick');
+
+        assert.strictEqual(selectionChangedHandler.callCount, 1, 'onSelectionChanged was called for the first time');
+        assert.deepEqual(selectionChangedHandler.args[0][0].addedItems, [{ text: 'item2', selected: true }], 'onSelectionChanged is called with selected item as added item');
+        assert.deepEqual(selectionChangedHandler.args[0][0].removedItems, [null], 'onSelectionChanged first called with null as removed item');
+
+        $item1.trigger('dxclick');
+
+        assert.strictEqual(selectionChangedHandler.callCount, 2, 'onSelectionChanged was called for the second time');
+        assert.deepEqual(selectionChangedHandler.args[1][0].addedItems, [{ text: 'item1', selected: true }], 'onSelectionChanged is called with selected item as added item');
+        assert.deepEqual(selectionChangedHandler.args[1][0].removedItems, [{ text: 'item2', selected: false }], 'onSelectionChanged is called with previously selected item as removed item');
+    });
+
+    QUnit.test('onSelectionChanged should have been called with correct arguments on unselect', function(assert) {
+        const items = [
+            { text: 'item1' },
+            { text: 'item2' },
+        ];
+        const selectionChangedHandler = sinon.stub();
+        const menuBase = createMenu({
+            items: items,
+            selectionMode: 'single',
+            selectByClick: true,
+            onSelectionChanged: selectionChangedHandler,
+        });
+        const $item = menuBase.element.find(`.${DX_MENU_ITEM_CLASS}`).eq(1);
+
+        $item.trigger('dxclick');
+
+        assert.strictEqual(selectionChangedHandler.callCount, 1, 'onSelectionChanged was called for the first time');
+        assert.deepEqual(selectionChangedHandler.args[0][0].addedItems, [{ text: 'item2', selected: true }], 'onSelectionChanged was called with selected item as added item');
+        assert.deepEqual(selectionChangedHandler.args[0][0].removedItems, [null], 'onSelectionChanged first called with null as removed item');
+
+        $item.trigger('dxclick');
+
+        assert.strictEqual(selectionChangedHandler.callCount, 2, 'onSelectionChanged was called for the second time');
+        assert.deepEqual(selectionChangedHandler.args[1][0].addedItems, [null], 'onSelectionChanged was called with null as added item');
+        assert.deepEqual(selectionChangedHandler.args[1][0].removedItems, [{ text: 'item2', selected: false }], 'onSelectionChanged was called with previously selected item as removed item');
+    });
+
     QUnit.test('Prevent selection item on click', function(assert) {
         const items = [
             { text: 'item1' },