Skip to content

Commit ec02640

Browse files
marker-daomarker dao ®
marker-dao
and
marker dao ®
authored
ButtonGroup/AIDialog: Prevent enter keydown event && Prevent update dialog state on hide
Co-authored-by: marker dao ® <[email protected]>
1 parent c51bba8 commit ec02640

File tree

7 files changed

+112
-43
lines changed

7 files changed

+112
-43
lines changed

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

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ const AI_DIALOG_COMMANDS_WITH_OPTIONS = ['translate', 'changeStyle', 'changeTone
5252

5353
const POPUP_MIN_WIDTH = 288;
5454
const POPUP_MAX_WIDTH = 460;
55+
const LOADINDICATOR_SIZE = 48;
5556
export const TEXT_AREA_MIN_HEIGHT = 64;
5657
export const TEXT_AREA_MAX_HEIGHT = 128;
5758
export const REPLACE_DROPDOWN_WIDTH = 150;
@@ -272,8 +273,8 @@ export default class AIDialog extends BaseDialog<AIDialogResult> {
272273

273274
const options: LoadIndicatorProperties = {
274275
_animationType: AnimationType.Sparkle,
275-
width: 48,
276-
height: 48,
276+
width: LOADINDICATOR_SIZE,
277+
height: LOADINDICATOR_SIZE,
277278
};
278279

279280
this._loadIndicator = new LoadIndicator($indicatorElement[0], options);
@@ -501,10 +502,13 @@ export default class AIDialog extends BaseDialog<AIDialogResult> {
501502
this._resultTextArea.option({ value });
502503
}
503504

504-
private _processCommandCompletion(dialogState: DialogState): void {
505+
private _processCommandCompletion(dialogState?: DialogState): void {
505506
this._abort = undefined;
506507
this._isAICommandExecuting = false;
507-
this._setDialogState(dialogState);
508+
509+
if (dialogState) {
510+
this._setDialogState(dialogState);
511+
}
508512
}
509513

510514
private _getAICommandCallbacks<T>(): RequestCallbacks<T> {
@@ -729,11 +733,11 @@ export default class AIDialog extends BaseDialog<AIDialogResult> {
729733
return super.show();
730734
}
731735

732-
hide(resultText: string, event: AIDialogResult['event']): void {
736+
hide(resultText: AIDialogResult['resultText'], event: AIDialogResult['event']): void {
733737
this.deferred?.resolve({ resultText, event });
734738

735739
this._abort?.();
736-
this._processCommandCompletion(this._getInitialDialogState());
740+
this._processCommandCompletion();
737741

738742
super.hide();
739743
}

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

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -58,23 +58,25 @@ abstract class BaseDialog<T = unknown> {
5858
// eslint-disable-next-line @typescript-eslint/no-unused-vars
5959
show(options?: unknown): Promise<T> | undefined {
6060
if (this._popup.option('visible')) {
61-
return;
61+
return undefined;
6262
}
6363

6464
this.deferred = Deferred<T>();
6565

66+
// eslint-disable-next-line @typescript-eslint/no-floating-promises
6667
this._popup.show();
6768

6869
return this.deferred.promise();
6970
}
7071

7172
// eslint-disable-next-line @typescript-eslint/no-unused-vars
72-
hide(_options?: unknown, _event?: unknown): void {
73+
hide(options?: unknown, event?: unknown): void {
74+
// eslint-disable-next-line @typescript-eslint/no-floating-promises
7375
this._popup.hide();
7476
}
7577

7678
popupOption(...args) {
77-
// @ts-expect-error
79+
// @ts-expect-error args is any
7880
return this._popup.option.apply(this._popup, args);
7981
}
8082
}

‎packages/devextreme/js/__internal/ui/html_editor/ui/m_formDialog.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -176,7 +176,8 @@ export default class FormDialog extends BaseDialog {
176176

177177
hide(formData, event): void {
178178
this.deferred?.resolve(formData, event);
179-
this._popup.hide();
179+
180+
super.hide();
180181
}
181182

182183
onHiding(): void {

‎packages/devextreme/js/__internal/ui/html_editor/utils/m_toolbar_helper.ts‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,10 @@ function getFormatHandlers(module) {
110110
function prepareAITextTrasformHandler(module) {
111111
return (options): void => {
112112
const {
113-
command, parentCommand, commandsMap, prompt,
113+
command,
114+
commandsMap,
115+
parentCommand,
116+
prompt,
114117
} = options;
115118

116119
const { quill } = module;
@@ -126,7 +129,9 @@ function prepareAITextTrasformHandler(module) {
126129
prompt,
127130
};
128131

129-
module.editorInstance.showAIDialog(aiDialogConfig)?.done(({
132+
const promise = module.editorInstance.showAIDialog(aiDialogConfig);
133+
134+
promise.done(({
130135
resultText,
131136
event: eventData,
132137
}: AIDialogResult) => {

‎packages/devextreme/js/__internal/ui/m_button_group.ts‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ import Widget from '@ts/core/widget/widget';
1616
import type { ItemRenderInfo } from './collection/collection_widget.base';
1717
import type { CollectionWidgetEditProperties } from './collection/m_collection_widget.edit';
1818

19-
const BUTTON_GROUP_CLASS = 'dx-buttongroup';
19+
export const BUTTON_GROUP_CLASS = 'dx-buttongroup';
2020
const BUTTON_GROUP_WRAPPER_CLASS = `${BUTTON_GROUP_CLASS}-wrapper`;
2121
const BUTTON_GROUP_ITEM_CLASS = `${BUTTON_GROUP_CLASS}-item`;
2222
const BUTTON_GROUP_FIRST_ITEM_CLASS = `${BUTTON_GROUP_CLASS}-first-item`;
@@ -121,6 +121,11 @@ class ButtonCollection extends CollectionWidget<ButtonCollectionProperties> {
121121
return this._focusTarget();
122122
}
123123

124+
_enterKeyHandler(e: KeyboardEvent): void {
125+
e.preventDefault();
126+
super._enterKeyHandler(e);
127+
}
128+
124129
_refreshContent(): void {
125130
this._prepareContent();
126131
this._renderContent();

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

Lines changed: 61 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import AIDialog, {
1313
TEXT_AREA_MIN_HEIGHT,
1414
TEXT_AREA_MAX_HEIGHT
1515
} from '__internal/ui/html_editor/ui/aiDialog';
16+
import { BUTTON_GROUP_CLASS } from '__internal/ui/m_button_group';
1617
import {
1718
ANIMATION_TYPE_CLASSES,
1819
LOADINDICATOR_CONTENT_CLASS,
@@ -105,7 +106,7 @@ function assertConfig(assert, config, expectations) {
105106
};
106107

107108
QUnit.module('AIDialog', {}, () => {
108-
QUnit.module('rendering and initial State', moduleConfig, () => {
109+
QUnit.module('rendering and initial state', moduleConfig, () => {
109110
QUnit.test('should render AI dialog content with correct values', function(assert) {
110111
showAIDialog(this);
111112

@@ -293,7 +294,7 @@ QUnit.module('AIDialog', {}, () => {
293294
});
294295
});
295296

296-
QUnit.module('Ask AI command', moduleConfig, () => {
297+
QUnit.module('askAI command', moduleConfig, () => {
297298
QUnit.test('should render correct UI', function(assert) {
298299
showAIDialog(this, {
299300
config: { currentCommand: 'askAI' }
@@ -758,6 +759,36 @@ QUnit.module('AIDialog', {}, () => {
758759

759760
assert.strictEqual($loadIndicatorContent.hasClass(ANIMATION_TYPE_CLASSES['sparkle']), true, 'animation type is sparkle');
760761
});
762+
763+
QUnit.test('should not change state on hide', function(assert) {
764+
showAIDialog(this);
765+
766+
this.setDialogState('generating');
767+
this.aiDialog.hide();
768+
769+
assert.strictEqual(getLoadIndicator(this.$element).length, 1, 'indicator is not removed');
770+
});
771+
772+
QUnit.test('should not throw an error if the Enter key was pressed on the replace button', function(assert) {
773+
const done = assert.async();
774+
775+
showAIDialog(this);
776+
777+
this.promise.then(() => {
778+
try {
779+
const $replaceButton = this.$element.find(`.${BUTTON_GROUP_CLASS}`);
780+
keyboardMock($replaceButton).press('enter');
781+
782+
assert.ok(true, 'There is no error');
783+
} catch(e) {
784+
assert.ok(false, `Error is raised: ${e.message}`);
785+
} finally {
786+
done();
787+
}
788+
});
789+
790+
this.resolve('');
791+
});
761792
});
762793

763794
QUnit.module('UI interactivity during generation', integrationModuleConfig, () => {
@@ -1227,41 +1258,41 @@ QUnit.module('AIDialog', {}, () => {
12271258
});
12281259
});
12291260
});
1230-
});
12311261

1232-
QUnit.module('compact', {
1233-
beforeEach: function() {
1234-
this.isCompactStub = sinon.stub(themes, 'isCompact').returns(true);
1235-
1236-
integrationModuleConfig.beforeEach.apply(this);
1237-
},
1238-
afterEach: function() {
1239-
integrationModuleConfig.afterEach.apply(this);
1262+
QUnit.module('compact theme', {
1263+
beforeEach() {
1264+
this.isCompactStub = sinon.stub(themes, 'isCompact').returns(true);
12401265

1241-
this.isCompactStub.restore();
1242-
}
1243-
}, () => {
1244-
QUnit.test('generate button should have special width', function(assert) {
1245-
showAIDialog(this, {
1246-
config: { currentCommand: 'askAI' },
1247-
});
1266+
integrationModuleConfig.beforeEach.apply(this);
1267+
},
1268+
afterEach() {
1269+
integrationModuleConfig.afterEach.apply(this);
12481270

1249-
const bottomToolbarItems = getBottomToolbarItems(this.aiDialogPopup);
1250-
const generateToolbarItem = getItemByName(bottomToolbarItems, 'generate');
1251-
const generateButtonOptions = generateToolbarItem.options;
1271+
this.isCompactStub.restore();
1272+
},
1273+
}, () => {
1274+
QUnit.test('generate button should have special width', function(assert) {
1275+
showAIDialog(this, {
1276+
config: { currentCommand: 'askAI' },
1277+
});
12521278

1253-
assert.strictEqual(generateButtonOptions.width, COMPACT_ACTION_BUTTON_WIDTH, 'width is specific');
1254-
});
1279+
const bottomToolbarItems = getBottomToolbarItems(this.aiDialogPopup);
1280+
const generateToolbarItem = getItemByName(bottomToolbarItems, 'generate');
1281+
const generateButtonOptions = generateToolbarItem.options;
12551282

1256-
QUnit.test('stop button should have special width', function(assert) {
1257-
showAIDialog(this, {
1258-
config: { currentCommand: 'translate' },
1283+
assert.strictEqual(generateButtonOptions.width, COMPACT_ACTION_BUTTON_WIDTH, 'width is specific');
12591284
});
12601285

1261-
const bottomToolbarItems = getBottomToolbarItems(this.aiDialogPopup);
1262-
const stopToolbarItem = getItemByName(bottomToolbarItems, 'stop');
1263-
const stopButtonOptions = stopToolbarItem.options;
1286+
QUnit.test('stop button should have special width', function(assert) {
1287+
showAIDialog(this, {
1288+
config: { currentCommand: 'translate' },
1289+
});
1290+
1291+
const bottomToolbarItems = getBottomToolbarItems(this.aiDialogPopup);
1292+
const stopToolbarItem = getItemByName(bottomToolbarItems, 'stop');
1293+
const stopButtonOptions = stopToolbarItem.options;
12641294

1265-
assert.strictEqual(stopButtonOptions.width, COMPACT_ACTION_BUTTON_WIDTH, 'width is specific');
1295+
assert.strictEqual(stopButtonOptions.width, COMPACT_ACTION_BUTTON_WIDTH, 'width is specific');
1296+
});
12661297
});
12671298
});

‎packages/devextreme/testing/tests/DevExpress.ui.widgets/buttonGroup.tests.js‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -458,6 +458,27 @@ QUnit.module('Events', () => {
458458
});
459459
});
460460

461+
QUnit.module('Keyboard navigation', () => {
462+
[
463+
{ key: 'enter', eventKey: 'Enter' },
464+
{ key: 'space', eventKey: ' ' },
465+
].forEach(({ key, eventKey }) => {
466+
QUnit.test(`${key} keydown event should be default prevented`, function(assert) {
467+
assert.expect(1);
468+
469+
const $element = $('#buttonGroup').dxButtonGroup({ items: [{ text: 'button 1' }] });
470+
471+
$element.on('keydown', (e) => {
472+
if(e.key === eventKey) {
473+
assert.ok(e.isDefaultPrevented(), `${key} event is prevented`);
474+
}
475+
});
476+
477+
keyboardMock($element).keyDown(key);
478+
});
479+
});
480+
});
481+
461482
if(devices.current().deviceType === 'desktop') {
462483
registerKeyHandlerTestHelper.runTests({
463484
createWidget: ($element, options) => $element.dxButtonGroup(

0 commit comments

Comments
 (0)