Skip to content

Commit

Permalink
Revert "Popup: place ‘Cancel’ button at the left of the ‘Done’ button(#…
Browse files Browse the repository at this point in the history
  • Loading branch information
alexanderPolosatov authored Oct 20, 2023
1 parent 217f64a commit d639788
Show file tree
Hide file tree
Showing 45 changed files with 57 additions and 65 deletions.
44 changes: 17 additions & 27 deletions packages/devextreme/js/ui/popup/ui.popup.js
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,6 @@ const POPUP_CONTENT_INHERIT_HEIGHT_CLASS = 'dx-popup-inherit-height';
const TOOLBAR_LABEL_CLASS = 'dx-toolbar-label';

const ALLOWED_TOOLBAR_ITEM_ALIASES = ['cancel', 'clear', 'done'];
const APPLY_VALUE_BUTTONS_ORDER = ['cancel', 'done'];

const BUTTON_DEFAULT_TYPE = 'default';
const BUTTON_NORMAL_TYPE = 'normal';
Expand All @@ -75,22 +74,15 @@ const BUTTON_CONTAINED_MODE = 'contained';
const IS_OLD_SAFARI = browser.safari && compareVersions(browser.version, [11]) < 0;
const HEIGHT_STRATEGIES = { static: '', inherit: POPUP_CONTENT_INHERIT_HEIGHT_CLASS, flex: POPUP_CONTENT_FLEX_HEIGHT_CLASS };


const sortApplyValueItems = (actionButtonsItems) => {
return actionButtonsItems.sort((a, b) => {
return APPLY_VALUE_BUTTONS_ORDER.indexOf(a.shortcut) - APPLY_VALUE_BUTTONS_ORDER.indexOf(b.shortcut);
});
};

const getButtonInfo = shortcut => {
const getButtonPlace = name => {

const device = devices.current();
const platform = device.platform;
let toolbar = 'bottom';
let location = 'before';

if(platform === 'ios') {
switch(shortcut) {
switch(name) {
case 'cancel':
toolbar = 'top';
break;
Expand All @@ -103,7 +95,7 @@ const getButtonInfo = shortcut => {
break;
}
} else if(platform === 'android') {
switch(shortcut) {
switch(name) {
case 'cancel':
location = 'after';
break;
Expand All @@ -115,8 +107,7 @@ const getButtonInfo = shortcut => {

return {
toolbar,
location,
shortcut
location
};
};

Expand Down Expand Up @@ -492,16 +483,19 @@ const Popup = Overlay.inherit({
},

_getToolbarItems: function(toolbar) {

const toolbarItems = this.option('toolbarItems');

const toolbarsItems = [];

this._toolbarItemClasses = [];

const currentPlatform = devices.current().platform;
let index = 0;
const applyValueButtonsInfo = [];

each(toolbarItems, (_, data) => {
const isShortcut = isDefined(data.shortcut);
const item = isShortcut ? getButtonInfo(data.shortcut) : data;
const item = isShortcut ? getButtonPlace(data.shortcut) : data;

if(isShortcut && currentPlatform === 'ios' && index < 2) {
item.toolbar = 'top';
Expand All @@ -510,17 +504,15 @@ const Popup = Overlay.inherit({

item.toolbar = data.toolbar || item.toolbar || 'top';

if(item?.toolbar === toolbar) {
if(item && item.toolbar === toolbar) {
if(isShortcut) {
extend(item, { location: data.location }, this._getToolbarItemByAlias(data));
if(APPLY_VALUE_BUTTONS_ORDER.includes(data.shortcut)) {
applyValueButtonsInfo.push({
shortcut: data.shortcut,
item
});
} else {
toolbarsItems.push(item);
}
}

const isLTROrder = currentPlatform === 'generic';

if((data.shortcut === 'done' && isLTROrder) || (data.shortcut === 'cancel' && !isLTROrder)) {
toolbarsItems.unshift(item);
} else {
toolbarsItems.push(item);
}
Expand All @@ -531,9 +523,7 @@ const Popup = Overlay.inherit({
toolbarsItems.push(this._getCloseButton());
}

const sortedApplyValueItems = sortApplyValueItems(applyValueButtonsInfo).map(item => item.item);

return toolbarsItems.concat(...sortedApplyValueItems);
return toolbarsItems;
},

_hasCloseButton() {
Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ test('DateBox should be closed by press esc key when today/cancel/apply button i
.pressKey('tab');

await t
.expect(dateBox.getPopup().getCancelButton().isFocused)
.expect(dateBox.getPopup().getApplyButton().isFocused)
.eql(true);

await t
Expand All @@ -184,7 +184,7 @@ test('DateBox should be closed by press esc key when today/cancel/apply button i
.pressKey('tab');

await t
.expect(dateBox.getPopup().getApplyButton().isFocused)
.expect(dateBox.getPopup().getCancelButton().isFocused)
.eql(true);

await t
Expand Down Expand Up @@ -264,7 +264,7 @@ test('dateBox keyboard navigation via `tab` key if applyValueMode is useButtons,
await t
.expect(dateBox.option('opened'))
.eql(true)
.expect(dateBox.getPopup().getCancelButton().isFocused)
.expect(dateBox.getPopup().getApplyButton().isFocused)
.ok();

await t
Expand All @@ -273,7 +273,7 @@ test('dateBox keyboard navigation via `tab` key if applyValueMode is useButtons,
await t
.expect(dateBox.option('opened'))
.eql(true)
.expect(dateBox.getPopup().getApplyButton().isFocused)
.expect(dateBox.getPopup().getCancelButton().isFocused)
.ok();

await t
Expand Down Expand Up @@ -327,7 +327,7 @@ test('dateBox keyboard navigation via `shift+tab` key if applyValueMode is useBu
await t
.expect(dateBox.option('opened'))
.eql(true)
.expect(dateBox.getPopup().getApplyButton().isFocused)
.expect(dateBox.getPopup().getCancelButton().isFocused)
.ok();

await t
Expand All @@ -336,7 +336,7 @@ test('dateBox keyboard navigation via `shift+tab` key if applyValueMode is useBu
await t
.expect(dateBox.option('opened'))
.eql(true)
.expect(dateBox.getPopup().getCancelButton().isFocused)
.expect(dateBox.getPopup().getApplyButton().isFocused)
.ok();

await t
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -278,7 +278,7 @@ test('DateRangeBox should be closed by press esc key when today/cancel/apply but
.pressKey('tab');

await t
.expect(dateRangeBox.getPopup().getCancelButton().isFocused)
.expect(dateRangeBox.getPopup().getApplyButton().isFocused)
.eql(true);

await t
Expand All @@ -305,7 +305,7 @@ test('DateRangeBox should be closed by press esc key when today/cancel/apply but
.pressKey('tab');

await t
.expect(dateRangeBox.getPopup().getApplyButton().isFocused)
.expect(dateRangeBox.getPopup().getCancelButton().isFocused)
.eql(true);

await t
Expand Down Expand Up @@ -651,9 +651,9 @@ test('DateRangeBox keyboard navigation via `tab` key if applyValueMode is useBut
.expect(dateRangeBox.getEndDateBox().isFocused)
.notOk()
.expect(dateRangeBox.getPopup().getApplyButton().isFocused)
.notOk()
.expect(dateRangeBox.getPopup().getCancelButton().isFocused)
.ok()
.expect(dateRangeBox.getPopup().getCancelButton().isFocused)
.notOk()
.expect(dateRangeBox.getPopup().getTodayButton().isFocused)
.notOk();

Expand All @@ -670,9 +670,9 @@ test('DateRangeBox keyboard navigation via `tab` key if applyValueMode is useBut
.expect(dateRangeBox.getEndDateBox().isFocused)
.notOk()
.expect(dateRangeBox.getPopup().getApplyButton().isFocused)
.ok()
.expect(dateRangeBox.getPopup().getCancelButton().isFocused)
.notOk()
.expect(dateRangeBox.getPopup().getCancelButton().isFocused)
.ok()
.expect(dateRangeBox.getPopup().getTodayButton().isFocused)
.notOk();

Expand Down Expand Up @@ -798,9 +798,9 @@ test('DateRangeBox keyboard navigation via `shift+tab` key if applyValueMode is
.expect(dateRangeBox.getPopup().getNavigatorNextButton().isFocused)
.notOk()
.expect(dateRangeBox.getPopup().getApplyButton().isFocused)
.ok()
.expect(dateRangeBox.getPopup().getCancelButton().isFocused)
.notOk()
.expect(dateRangeBox.getPopup().getCancelButton().isFocused)
.ok()
.expect(dateRangeBox.getPopup().getTodayButton().isFocused)
.notOk();

Expand All @@ -823,9 +823,9 @@ test('DateRangeBox keyboard navigation via `shift+tab` key if applyValueMode is
.expect(dateRangeBox.getPopup().getNavigatorNextButton().isFocused)
.notOk()
.expect(dateRangeBox.getPopup().getApplyButton().isFocused)
.notOk()
.expect(dateRangeBox.getPopup().getCancelButton().isFocused)
.ok()
.expect(dateRangeBox.getPopup().getCancelButton().isFocused)
.notOk()
.expect(dateRangeBox.getPopup().getTodayButton().isFocused)
.notOk();

Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ const COLORVIEW_HEX_INPUT_SELECTOR = '.dx-colorview-label-hex .dx-texteditor-inp
const COLORVIEW_APPLY_BUTTON_SELECTOR = '.dx-colorview-apply-button';
const CLEAR_BUTTON_AREA_SELECTOR = '.dx-clear-button-area';
const COLOR_VIEW_PALETTE_HANDLE_SELECTOR = '.dx-colorview-palette-handle';
const COLOR_VIEW_CANCEL_BUTTON_SELECTOR = '.dx-colorview-cancel-button';
const BUTTON_SELECTOR = '.dx-button';
const TEXTBOX_SELECTOR = '.dx-textbox';

Expand Down Expand Up @@ -879,16 +880,16 @@ QUnit.module('keyboard navigation', {
assert.ok($(`.${COLORVIEW_CLASS}`).hasClass(STATE_FOCUSED_CLASS));
});

QUnit.test('pressing tab + shift should set focus on apply button in popup', function(assert) {
QUnit.test('pressing tab + shift should set focus on cancel button in popup', function(assert) {
this.instance.option({
opened: true,
applyValueMode: 'useButtons',
});
this.keyboard.keyDown('tab', { shiftKey: true });

const $applyButton = getColorBoxOverlayContent().find(COLORVIEW_APPLY_BUTTON_SELECTOR);
const $cancelButton = getColorBoxOverlayContent().find(COLOR_VIEW_CANCEL_BUTTON_SELECTOR);

assert.ok($applyButton.hasClass(STATE_FOCUSED_CLASS));
assert.ok($cancelButton.hasClass(STATE_FOCUSED_CLASS));
});

QUnit.test('pressing tab should set focus on first item in popup with custom items', function(assert) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ const TEXTEDITOR_EMPTY_CLASS = 'dx-texteditor-empty';
const CALENDAR_CELL_CLASS = 'dx-calendar-cell';
const CALENDAR_CONTOURED_CELL_CLASS = 'dx-calendar-contoured-date';
const APPLY_BUTTON_SELECTOR = '.dx-popup-done.dx-button';
const CANCEL_BUTTON_SELECTOR = '.dx-popup-cancel.dx-button';
const CALENDAR_NAVIGATOR_PREVIOUS_VIEW_CLASS = 'dx-calendar-navigator-previous-view';
const BUTTON_SELECTOR = '.dx-button';
const TEXTBOX_SELECTOR = '.dx-textbox';
Expand Down Expand Up @@ -4219,7 +4220,7 @@ if(devices.real().deviceType === 'desktop') {
assert.ok($prevButton.hasClass(STATE_FOCUSED_CLASS));
});

QUnit.test('pressing tab + shift should set focus on apply button in popup', function(assert) {
QUnit.test('pressing tab + shift should set focus on cancel button in popup', function(assert) {
this.reinit({
opened: true,
applyValueMode: 'useButtons',
Expand All @@ -4231,8 +4232,8 @@ if(devices.real().deviceType === 'desktop') {
shiftKey: true
}));

const $applyButton = this.getPopupContent().parent().find(APPLY_BUTTON_SELECTOR);
assert.ok($applyButton.hasClass(STATE_FOCUSED_CLASS));
const $cancelButton = this.getPopupContent().parent().find(CANCEL_BUTTON_SELECTOR);
assert.ok($cancelButton.hasClass(STATE_FOCUSED_CLASS));
});

QUnit.test('pressing tab should set focus on first item in popup with custom items', function(assert) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4772,8 +4772,8 @@ if(devices.real().deviceType === 'desktop') {
shiftKey: true
}));

const $doneButton = this.dateBox._popup.$wrapper().find('.dx-button.dx-popup-done');
assert.ok($doneButton.hasClass('dx-state-focused'), 'cancel button is focused');
const $cancelButton = this.dateBox._popup.$wrapper().find('.dx-button.dx-popup-cancel');
assert.ok($cancelButton.hasClass('dx-state-focused'), 'cancel button is focused');
});

QUnit.test('pressing tab should set focus on calendar prev button in popup', function(assert) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -801,12 +801,12 @@ if(devices.real().deviceType === 'desktop') {
QUnit.testInActiveWindow('the first popup element should be focused on the \'tab\' key press if the input is focused', function(assert) {
this.instance.open();
this.triggerKeyPress(this.$input, TAB_KEY_CODE);
assert.ok(this.$cancelButton.hasClass('dx-state-focused'), 'the first popup element is focused');
assert.ok(this.$doneButton.hasClass('dx-state-focused'), 'the first popup element is focused');
});

QUnit.testInActiveWindow('the input should be focused on the \'tab\' key press if the last element is focused', function(assert) {
this.instance.open();
this.triggerKeyPress(this.$doneButton, TAB_KEY_CODE);
this.triggerKeyPress(this.$cancelButton, TAB_KEY_CODE);
assert.ok(this.$element.hasClass('dx-state-focused'), 'the input is focused');
});

Expand Down Expand Up @@ -866,21 +866,21 @@ if(devices.real().deviceType === 'desktop') {

QUnit.testInActiveWindow('the input should be focused on the \'tab+shift\' key press if the first element is focused', function(assert) {
this.instance.open();
this.triggerKeyPress(this.$cancelButton, TAB_KEY_CODE, true);
this.triggerKeyPress(this.$doneButton, TAB_KEY_CODE, true);
assert.ok(this.$element.hasClass('dx-state-focused'), 'the input is focused');
});

QUnit.testInActiveWindow('the last popup element should be focused on the \'tab+shift\' key press if the input is focused', function(assert) {
this.instance.open();
this.triggerKeyPress(this.$input, TAB_KEY_CODE, true);
assert.ok(this.$doneButton.hasClass('dx-state-focused'), 'the last popup element is focused');
assert.ok(this.$cancelButton.hasClass('dx-state-focused'), 'the last popup element is focused');
});

QUnit.testInActiveWindow('default event should be prevented on the tab key press if the input is focused', function(assert) {
this.instance.open();
const spy = sinon.spy();
this.$doneButton.on('keydown', spy);
this.triggerKeyPress(this.$doneButton, TAB_KEY_CODE);
this.$cancelButton.on('keydown', spy);
this.triggerKeyPress(this.$cancelButton, TAB_KEY_CODE);
assert.ok(spy.args[0][0].isDefaultPrevented(), 'default is prevented');
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5427,7 +5427,7 @@ QUnit.module('keyboard navigation "TAB" button', moduleSetup, () => {
assert.equal(caret.start, caret.end, 'the input has no selection');
});

QUnit.testInActiveWindow('the "tab" key press should focus the "cancel" button if the input is focused', function(assert) {
QUnit.testInActiveWindow('the "tab" key press should focus the "apply" button if the input is focused', function(assert) {
if(devices.real().deviceType !== 'desktop') {
assert.ok(true, 'desktop specific test');
return;
Expand All @@ -5440,14 +5440,14 @@ QUnit.module('keyboard navigation "TAB" button', moduleSetup, () => {
opened: true
});
const instance = $element.dxSelectBox('instance');
const $cancelButton = instance._popup.$wrapper().find('.dx-popup-cancel.dx-button');
const $applyButton = instance._popup.$wrapper().find('.dx-popup-done.dx-button');

keyboardMock($element.find(toSelector(TEXTEDITOR_INPUT_CLASS)), true)
.focus()
.keyDown('tab');

assert.ok(instance.option('opened'), 'popup is still opened');
assert.ok($cancelButton.hasClass(STATE_FOCUSED_CLASS), 'the apply button is focused');
assert.ok($applyButton.hasClass(STATE_FOCUSED_CLASS), 'the apply button is focused');
});
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2580,7 +2580,7 @@ QUnit.module('keyboard navigation', {
assert.deepEqual(this.instance.option('value'), expectedValue, 'the value is correct');
});

QUnit.testInActiveWindow('the \'cancel\' button should be focused on the \'tab\' key press if the input is focused and showSelectionControls if false (T389453)', function(assert) {
QUnit.testInActiveWindow('the \'apply\' button should be focused on the \'tab\' key press if the input is focused and showSelectionControls if false (T389453)', function(assert) {
if(devices.real().deviceType !== 'desktop') {
assert.ok(true, 'desktop specific test');
return;
Expand All @@ -2595,8 +2595,8 @@ QUnit.module('keyboard navigation', {
.focus()
.press('tab');

const $cancelButton = this.instance._popup.$wrapper().find('.dx-button.dx-popup-cancel');
assert.ok($cancelButton.hasClass('dx-state-focused'), 'the apply button is focused');
const $applyButton = this.instance._popup.$wrapper().find('.dx-button.dx-popup-done');
assert.ok($applyButton.hasClass('dx-state-focused'), 'the apply button is focused');
});

QUnit.testInActiveWindow('toolbar button should be focused on the "tab" key press if the input is focused and showSelectionControls is enabled', function(assert) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -422,7 +422,7 @@ module('Appointment popup buttons', moduleConfig, () => {
scheduler.tooltip.clickOnItem();

const popup = scheduler.appointmentPopup;
assert.ok(popup.hasToolbarButtonsInSection(TOOLBAR_BOTTOM_LOCATION, SECTION_AFTER, [ CANCEL_BUTTON, DONE_BUTTON]), 'the \'Cancel\' and \'Done\' buttons are located in the \'after\' section');
assert.ok(popup.hasToolbarButtonsInSection(TOOLBAR_BOTTOM_LOCATION, SECTION_AFTER, [DONE_BUTTON, CANCEL_BUTTON]), 'the \'Cancel\' and \'Done\' buttons are located in the \'after\' section');
} finally {
this.realDeviceMock.restore();
}
Expand Down
Loading

0 comments on commit d639788

Please sign in to comment.