HtmlEditor: Move the execution abort to Popup.onHiding && Skip execution in onValueChanged

Author: marker-dao

packages/devextreme/js/__internal/ui/html_editor/ui/aiDialog.ts

@@ -103,6 +103,8 @@ export default class AIDialog extends BaseDialog<AIDialogResult> {
 
   private _commandChangeSuppressed = false;
 
+  private _commandOptionSuppressed = false;
+
   private _commandOptionsList?: string[];
 
   private _commandSelectBox!: dxSelectBox;
@@ -163,7 +165,10 @@ export default class AIDialog extends BaseDialog<AIDialogResult> {
         at: 'center',
         of: this._$container,
       },
-      ...this._popupUserConfig,
+      onHiding: () => {
+        this._processCommandCompletion();
+      },
+      ...this._popupConfig,
     }) as PopupProperties;
   }
 
@@ -199,12 +204,17 @@ export default class AIDialog extends BaseDialog<AIDialogResult> {
 
   protected _renderOptionSelectBox($container: dxElementWrapper): void {
     const $optionSelectBox = $('<div>').appendTo($container);
+
     this._optionSelectBox = new SelectBox($optionSelectBox.get(0), {
       items: this._commandOptionsList,
       value: this._currentOption ?? this._commandOptionsList?.[0],
       visible: this._isCommandWithOptionsSelected(),
       onInitialized: this._addEscapeHandler.bind(this),
       onValueChanged: ({ value }): void => {
+        if (this._commandOptionSuppressed) {
+          return;
+        }
+
         this._currentOption = value;
 
         if (this._isOpen() && value) {
@@ -524,6 +534,7 @@ export default class AIDialog extends BaseDialog<AIDialogResult> {
   }
 
   private _processCommandCompletion(dialogState?: DialogState): void {
+    this._abort?.();
     this._abort = undefined;
     this._isAICommandExecuting = false;
 
@@ -568,7 +579,6 @@ export default class AIDialog extends BaseDialog<AIDialogResult> {
   }
 
   private _stopAICommandExecution(): void {
-    this._abort?.();
     this._processCommandCompletion(this._getInitialDialogState());
   }
 
@@ -599,12 +609,14 @@ export default class AIDialog extends BaseDialog<AIDialogResult> {
   private _refreshOptionSelectBox(): void {
     const hasOptions = this._isCommandWithOptionsSelected();
 
+    this._commandOptionSuppressed = true;
     this._optionSelectBox.option({
       disabled: this._isAICommandExecuting,
       visible: hasOptions,
       items: this._commandOptionsList ?? [],
       value: this._currentOption ?? this._commandOptionsList?.[0],
     });
+    this._commandOptionSuppressed = false;
   }
 
   private _setTextAreasInitialState(): void {
@@ -734,7 +746,6 @@ export default class AIDialog extends BaseDialog<AIDialogResult> {
   }
 
   updateAIIntegration(aiIntegration: AIIntegration): void {
-    this._abort?.();
     this._processCommandCompletion(this._getInitialDialogState());
     this._aiIntegration = aiIntegration;
     this._executeAICommand();
@@ -765,10 +776,6 @@ export default class AIDialog extends BaseDialog<AIDialogResult> {
 
   hide(resultText: AIDialogResult['resultText'], event: AIDialogResult['event']): void {
     this.deferred?.resolve({ resultText, event });
-
-    this._abort?.();
-    this._processCommandCompletion();
-
     super.hide();
   }
 }

packages/devextreme/js/__internal/ui/html_editor/ui/formDialog.ts

@@ -107,7 +107,7 @@ export default class FormDialog extends BaseDialog {
           },
         },
       ],
-      ...this._popupUserConfig,
+      ...this._popupConfig,
     }) as PopupProperties;
   }
 

packages/devextreme/js/__internal/ui/html_editor/ui/m_baseDialog.ts

@@ -11,15 +11,15 @@ const DROPDOWN_EDITOR_OVERLAY_CLASS = 'dx-dropdowneditor-overlay';
 abstract class BaseDialog<T = unknown> {
   _$container: dxElementWrapper;
 
-  _popupUserConfig?: PopupProperties;
+  _popupConfig?: PopupProperties;
 
   _popup!: Popup;
 
   deferred?: DeferredObj<T>;
 
   constructor($container: dxElementWrapper, popupConfig?: PopupProperties) {
     this._$container = $container;
-    this._popupUserConfig = popupConfig;
+    this._popupConfig = popupConfig;
 
     this._renderPopup();
   }

packages/devextreme/js/__internal/ui/popup/m_popup.ts

@@ -58,7 +58,7 @@ const DISABLED_STATE_CLASS = 'dx-state-disabled';
 const POPUP_DRAGGABLE_CLASS = 'dx-popup-draggable';
 
 const POPUP_TITLE_CLASS = 'dx-popup-title';
-const POPUP_TITLE_CLOSEBUTTON_CLASS = 'dx-closebutton';
+export const POPUP_TITLE_CLOSEBUTTON_CLASS = 'dx-closebutton';
 
 const POPUP_BOTTOM_CLASS = 'dx-popup-bottom';
 

packages/devextreme/testing/tests/DevExpress.ui.widgets.htmlEditor/htmlEditorParts/aiDialog.tests.js

@@ -868,6 +868,19 @@ QUnit.module('AIDialog', () => {
 
             this.resolve('');
         });
+
+        QUnit.test('changing the dialog state does not trigger additional command execution if the command has options', function(assert) {
+            this.showDialog({ currentCommand: 'changeStyle', currentCommandOption: 'business' });
+            const commandSelectBox = getCommandSelectBoxInstance(this.$element);
+
+            assert.ok(this.sendRequestStub.calledOnce, 'command execution is called once');
+
+            commandSelectBox.option('value', 'askAI');
+            assert.ok(this.sendRequestStub.calledOnce, 'command execution is called once');
+
+            commandSelectBox.option('value', 'changeStyle');
+            assert.strictEqual(this.sendRequestStub.callCount, 2, 'command execution is called twice');
+        });
     });
 
     QUnit.module('UI interactivity during generation', integrationModuleConfig, () => {

packages/devextreme/testing/tests/DevExpress.ui.widgets.htmlEditor/htmlEditorParts/aiDialogIntegration.tests.js

@@ -14,8 +14,10 @@ import {
     getResultTextAreaValue,
 } from '../../../helpers/aiDialog.js';
 import { AI_DIALOG_CLASS } from '__internal/ui/html_editor/ui/aiDialog';
+import { POPUP_TITLE_CLOSEBUTTON_CLASS } from '__internal/ui/popup/m_popup';
 import { DX_MENU_ITEM_CLASS } from '__internal/ui/menu/m_menu';
 import uiErrors from 'ui/widget/ui.errors';
+import keyboardMock from '../../../helpers/keyboardMock.js';
 
 const MENU_ITEM_CLASS = 'dx-menu-item';
 const MENU_CLASS = 'dx-menu';
@@ -28,7 +30,7 @@ const setupHtmlEditorWithAi = (config) => {
         toolbar: {
             items: [{
                 name: 'ai',
-                commands: ['summarize']
+                commands: ['summarize'],
             }],
         },
         ...config
@@ -43,6 +45,18 @@ const getAIDialogElement = ($htmlEditor) => {
     return $htmlEditor.find(`.${AI_DIALOG_CLASS}`);
 };
 
+const getPopup = (htmlEditor) => {
+    return getAIDialog(htmlEditor)._popup;
+};
+
+const getOverlayContentElement = (htmlEditor) => {
+    return $(getPopup(htmlEditor).content()).parent();
+};
+
+const getCloseButtonElement = (htmlEditor) => {
+    return getOverlayContentElement(htmlEditor).find(`.${POPUP_TITLE_CLOSEBUTTON_CLASS}`);
+};
+
 QUnit.module('AI dialog integration', () => {
     QUnit.module('toolbar', () => {
         QUnit.test('Should pass correct payload to dialog on item click', function(assert) {
@@ -507,4 +521,41 @@ QUnit.module('AI dialog integration', () => {
             assert.strictEqual($menuItem.attr('aria-label'), 'AI Assistant toolbar item', 'menu item has correct aria-label');
         });
     });
+
+    QUnit.module('command execution abort on hiding', {
+        beforeEach() {
+            this.abortSpy = sinon.spy();
+            this.commandSpy = sinon.stub().returns(this.abortSpy);
+            const aiIntegration = { summarize: this.commandSpy };
+            this.htmlEditor = setupHtmlEditorWithAi({ aiIntegration });
+        },
+        afterEach() {
+            sinon.restore();
+        },
+    }, () => {
+        [
+            {
+                name: 'click outside',
+                action() { $(document).trigger('dxpointerdown'); },
+            },
+            {
+                name: 'click on the close button',
+                action() { getCloseButtonElement(this.htmlEditor).trigger('dxclick'); },
+            },
+            {
+                name: 'esc keydown',
+                action() { keyboardMock(getOverlayContentElement(this.htmlEditor)).press('escape'); },
+            },
+        ].forEach(({ name, action }) => {
+            QUnit.test(`should abort on ${name}`, function(assert) {
+                openAIDialog($(this.htmlEditor.$element()));
+
+                assert.ok(this.commandSpy.calledOnce, 'command execution is started');
+
+                action.call(this);
+
+                assert.ok(this.abortSpy.calledOnce, 'abort function is called once');
+            });
+        });
+    });
 });