From a3d98dae68b97aeb7c36ca93c31f067535cbe8fd Mon Sep 17 00:00:00 2001 From: Aaron Bohy Date: Thu, 11 Apr 2019 12:52:35 +0000 Subject: [PATCH] [FIX] web: do not open datepicker on focus Before this rev., the datepicker was opened when a date(time) field was focused (e.g. with keyboard navigation). We don't want this behavior anymore, and we want the datepicker to open only when the user clicks in the input. This rev. makes this change. Moreover, this allowed to refined the keyboard navigation (ESC) in editable list views: when a datepicker is opened, and the user presses ESC, we close the datepicker, but we keep the row in edition, whereas before this rev., the row was switched to readonly. closes odoo/odoo#32408 Signed-off-by: Martin Geubelle (mge) --- .../web/static/src/js/widgets/date_picker.js | 35 ++++++++++++++++--- .../static/tests/fields/basic_fields_tests.js | 28 +++++++++------ .../relational_fields/field_one2many_tests.js | 5 --- .../static/tests/helpers/test_utils_dom.js | 19 +++++----- addons/web/static/tests/views/list_tests.js | 34 ++++++++++-------- 5 files changed, 78 insertions(+), 43 deletions(-) diff --git a/addons/web/static/src/js/widgets/date_picker.js b/addons/web/static/src/js/widgets/date_picker.js index 57c935905e5..75c52ce81a7 100644 --- a/addons/web/static/src/js/widgets/date_picker.js +++ b/addons/web/static/src/js/widgets/date_picker.js @@ -14,8 +14,10 @@ var DateWidget = Widget.extend({ events: { 'change.datetimepicker': 'changeDatetime', 'change .o_datepicker_input': 'changeDatetime', + 'click input': '_onInputClicked', 'input input': '_onInput', 'keydown': '_onKeydown', + 'hide.datetimepicker': '_onDateTimePickerHide', 'show.datetimepicker': '_onDateTimePickerShow', }, /** @@ -50,10 +52,13 @@ var DateWidget = Widget.extend({ }, widgetParent: 'body', keyBinds: null, - allowInputToggle: true, }, options || {}); this.__libInput = 0; + // tempusdominus doesn't offer any elegant way to check whether the + // datepicker is open or not, so we have to listen to hide/show events + // and manually keep track of the 'open' state + this.__isOpen = false; }, /** * @override @@ -228,6 +233,12 @@ var DateWidget = Widget.extend({ // Handlers //-------------------------------------------------------------------------- + /** + * @private + */ + _onDateTimePickerHide: function () { + this.__isOpen = false; + }, /** * Reacts to the datetimepicker being shown * Could set/verify our widget value @@ -236,6 +247,7 @@ var DateWidget = Widget.extend({ * @private */ _onDateTimePickerShow: function () { + this.__isOpen = true; if (this.$input.val().length !== 0 && this.isValid()) { this.$input.select(); } @@ -246,9 +258,15 @@ var DateWidget = Widget.extend({ */ _onKeydown: function (ev) { if (ev.which === $.ui.keyCode.ESCAPE) { - this.__libInput++; - this.$el.datetimepicker('hide'); - this.__libInput--; + if (this.__isOpen) { + // we don't want any other effects than closing the datepicker, + // like leaving the edition of a row in editable list view + ev.stopImmediatePropagation(); + this.__libInput++; + this.$el.datetimepicker('hide'); + this.__libInput--; + this.focus(); + } } }, /** @@ -264,6 +282,15 @@ var DateWidget = Widget.extend({ ev.stopImmediatePropagation(); } }, + /** + * @private + */ + _onInputClicked: function () { + this.__libInput++; + this.$el.datetimepicker('toggle'); + this.__libInput--; + this.focus(); + }, }); var DateTimeWidget = DateWidget.extend({ diff --git a/addons/web/static/tests/fields/basic_fields_tests.js b/addons/web/static/tests/fields/basic_fields_tests.js index c184b0c9382..f0bd135973c 100644 --- a/addons/web/static/tests/fields/basic_fields_tests.js +++ b/addons/web/static/tests/fields/basic_fields_tests.js @@ -2860,10 +2860,7 @@ QUnit.module('basic_fields', { View: FormView, model: 'partner', data: this.data, - arch:'
' + - ' ' + // Do not focus on the date field right away - '' + - '', + arch: '
', res_id: 1, viewOptions: { mode: 'edit', @@ -2879,7 +2876,8 @@ QUnit.module('basic_fields', { done(); } }); - form.$('.o_input[name="date"]').mouseenter().trigger('focus'); + + testUtils.dom.openDatepicker(form.$('.o_datepicker')); form.destroy(); }); @@ -2928,13 +2926,13 @@ QUnit.module('basic_fields', { QUnit.module('FieldDatetime'); QUnit.test('datetime field in form view', async function (assert) { - assert.expect(6); + assert.expect(7); var form = await createView({ View: FormView, model: 'partner', data: this.data, - arch:'
', + arch: '
', res_id: 1, translateParameters: { // Avoid issues due to localization formats date_format: '%m/%d/%Y', @@ -2955,8 +2953,14 @@ QUnit.module('basic_fields', { await testUtils.form.clickEdit(form); assert.strictEqual(form.$('.o_datepicker_input').val(), expectedDateString, 'the datetime should be correct in edit mode'); + + // datepicker should not open on focus + assert.containsNone($('body'), '.bootstrap-datetimepicker-widget'); + + testUtils.dom.openDatepicker(form.$('.o_datepicker')); + assert.containsOnce($('body'), '.bootstrap-datetimepicker-widget'); + // select 22 February at 8:23:33 - assert.ok($('.bootstrap-datetimepicker-widget').length, 'datepicker should be open'); await testUtils.dom.click($('.bootstrap-datetimepicker-widget .picker-switch').first()); await testUtils.dom.click($('.bootstrap-datetimepicker-widget .picker-switch:eq(1)')); await testUtils.dom.click($('.bootstrap-datetimepicker-widget .year:contains(2017)')); @@ -3038,7 +3042,7 @@ QUnit.module('basic_fields', { }); QUnit.test('datetime field in editable list view', async function (assert) { - assert.expect(8); + assert.expect(9); var list = await createView({ View: ListView, @@ -3074,8 +3078,11 @@ QUnit.module('basic_fields', { assert.strictEqual(list.$('input.o_datepicker_input').val(), expectedDateString, 'the date should be correct in edit mode'); + assert.containsNone($('body'), '.bootstrap-datetimepicker-widget'); + testUtils.dom.openDatepicker(list.$('.o_datepicker')); + assert.containsOnce($('body'), '.bootstrap-datetimepicker-widget'); + // select 22 February at 8:23:33 - assert.ok($('.bootstrap-datetimepicker-widget').length, 'datepicker should be open'); await testUtils.dom.click($('.bootstrap-datetimepicker-widget .picker-switch').first()); await testUtils.dom.click($('.bootstrap-datetimepicker-widget .picker-switch:eq(1)')); await testUtils.dom.click($('.bootstrap-datetimepicker-widget .year:contains(2017)')); @@ -3251,6 +3258,7 @@ QUnit.module('basic_fields', { }); await testUtils.form.clickCreate(form); + testUtils.dom.openDatepicker(form.$('.o_datepicker')); $.each($('.day:last-child(),.day:nth-child(2)'), function (index, value) { assert.hasClass(value, 'disabled', 'first and last days must be disabled'); }); diff --git a/addons/web/static/tests/fields/relational_fields/field_one2many_tests.js b/addons/web/static/tests/fields/relational_fields/field_one2many_tests.js index 0a96e07310e..4eb02b0c380 100644 --- a/addons/web/static/tests/fields/relational_fields/field_one2many_tests.js +++ b/addons/web/static/tests/fields/relational_fields/field_one2many_tests.js @@ -3428,11 +3428,6 @@ QUnit.module('fields', {}, function () { await testUtils.dom.click($('.bootstrap-datetimepicker-widget .year:contains(2017)')); await testUtils.dom.click($('.bootstrap-datetimepicker-widget .month').eq(1)); await testUtils.dom.click($('.day:contains(22)')); - // trigger the blur of input before the click on Save, to closely mock what actually - // happens when we really click on Save - // this is required to close the datepicker before removing the input from the DOM - // (because of the re-rendering in readonly), otherwise tempusdominus crashes - await testUtils.dom.triggerEvents(form.$('.o_datepicker_input'), 'blur'); await testUtils.form.clickSave(form); assert.verifySteps(['read', 'read', 'onchange', 'write', 'read', 'read']); diff --git a/addons/web/static/tests/helpers/test_utils_dom.js b/addons/web/static/tests/helpers/test_utils_dom.js index 315a00cbfe7..4c1ef8fbbcc 100644 --- a/addons/web/static/tests/helpers/test_utils_dom.js +++ b/addons/web/static/tests/helpers/test_utils_dom.js @@ -166,15 +166,6 @@ function triggerKeypressEvent(char) { return $('body').trigger($.Event('keypress', { which: keycode, keyCode: keycode })); } -/** - * Opens the datepicker of a given element. - * - * @param {jQuery} $datepickerEl element to which a datepicker is attached - */ -function openDatepicker($datepickerEl) { - $datepickerEl.find('.o_datepicker_input').trigger('focus.datetimepicker'); -} - /** * Click on a specified element. If the option first or last is not specified, * this method also check the unicity and the visibility of the target. @@ -277,6 +268,16 @@ function triggerEvents($el, events) { } +/** + * Opens the datepicker of a given element. + * + * @param {jQuery} $datepickerEl element to which a datepicker is attached + */ +function openDatepicker($datepickerEl) { + click($datepickerEl.find('.o_datepicker_input')); +} + + return { triggerKeypressEvent: triggerKeypressEvent, triggerMouseEvent: triggerMouseEvent, diff --git a/addons/web/static/tests/views/list_tests.js b/addons/web/static/tests/views/list_tests.js index 5a8e19b8fb7..f10771a8e66 100644 --- a/addons/web/static/tests/views/list_tests.js +++ b/addons/web/static/tests/views/list_tests.js @@ -299,7 +299,7 @@ QUnit.module('Views', { }); QUnit.test('editable list datetimepicker destroy widget (edition)', async function (assert) { - assert.expect(6); + assert.expect(7); var eventPromise = testUtils.makeTestPromise(); var list = await createView({ @@ -317,9 +317,13 @@ QUnit.module('Views', { await testUtils.fields.triggerKeydown(list.$('.o_datepicker_input'), 'escape'); - assert.containsNone(list, '.o_selected_row'); + assert.containsOnce(list, '.o_selected_row'); assert.containsNone($('body'), '.bootstrap-datetimepicker-widget'); + await testUtils.fields.triggerKeydown($(document.activeElement), 'escape'); + + assert.containsNone(list, '.o_selected_row'); + eventPromise.resolve(); } }); @@ -328,13 +332,14 @@ QUnit.module('Views', { assert.containsNone(list, '.o_selected_row'); await testUtils.dom.click(list.$('.o_data_cell:first')); + await testUtils.dom.openDatepicker(list.$('.o_datepicker')); await eventPromise; list.destroy(); }); QUnit.test('editable list datetimepicker destroy widget (new line)', async function (assert) { - assert.expect(7); + assert.expect(10); var eventPromise = testUtils.makeTestPromise(); var list = await createView({ @@ -347,22 +352,20 @@ QUnit.module('Views', { }); list.$el.on({ 'show.datetimepicker': async function () { - assert.equal($('.bootstrap-datetimepicker-widget').length, 1, - 'The datetimepicker is open'); - - assert.equal(list.$('.o_data_row').length, 5, - 'There should be 5 rows'); - - assert.equal(list.$('.o_selected_row').length, 1, - 'One row in edit mode'); + assert.containsOnce($('body'), '.bootstrap-datetimepicker-widget'); + assert.containsN(list, '.o_data_row', 5); + assert.containsOnce(list, '.o_selected_row'); await testUtils.fields.triggerKeydown(list.$('.o_datepicker_input'), 'escape'); - assert.equal(list.$('.o_data_row').length, 4, - 'There should be 4 rows'); + assert.containsNone($('body'), '.bootstrap-datetimepicker-widget'); + assert.containsN(list, '.o_data_row', 5); + assert.containsOnce(list, '.o_selected_row'); - assert.equal(list.$('.o_selected_row').length, 0, - 'No row should be in edit mode'); + await testUtils.fields.triggerKeydown($(document.activeElement), 'escape'); + + assert.containsN(list, '.o_data_row', 4); + assert.containsNone(list, '.o_selected_row'); eventPromise.resolve(); } @@ -374,6 +377,7 @@ QUnit.module('Views', { 'No row should be in edit mode'); await testUtils.dom.click(list.$buttons.find('.o_list_button_add')); + await testUtils.dom.openDatepicker(list.$('.o_datepicker')); await eventPromise; list.destroy();