diff --git a/addons/web/static/src/js/fields/abstract_field.js b/addons/web/static/src/js/fields/abstract_field.js index 9f977d41392..4948a3e3c31 100644 --- a/addons/web/static/src/js/fields/abstract_field.js +++ b/addons/web/static/src/js/fields/abstract_field.js @@ -344,7 +344,7 @@ var AbstractField = Widget.extend({ _setValue: function (value) { // we try to avoid doing useless work, if the value given has not // changed. Note that we compare the unparsed values. - if (this.lastSetValue === value) { + if (this.lastSetValue === value || (this.value === false && value === '')) { return; } this.lastSetValue = value; diff --git a/addons/web/static/src/js/fields/basic_fields.js b/addons/web/static/src/js/fields/basic_fields.js index a462efee6e0..e600954b967 100644 --- a/addons/web/static/src/js/fields/basic_fields.js +++ b/addons/web/static/src/js/fields/basic_fields.js @@ -55,6 +55,23 @@ var DebouncedField = AbstractField.extend({ } }, + //-------------------------------------------------------------------------- + // Public + //-------------------------------------------------------------------------- + + /** + * Debounced fields do not notify the rest of the environment immediately of + * their new state, so it is necessary to actually get their value when this + * method is called. + * + * @override + */ + commitChanges: function () { + if (this.mode === 'edit') { + this._setValue(this._getValue()); + } + }, + //-------------------------------------------------------------------------- // Private //-------------------------------------------------------------------------- diff --git a/addons/web/static/src/js/fields/relational_fields.js b/addons/web/static/src/js/fields/relational_fields.js index 2a3d5e01125..28d790b16c4 100644 --- a/addons/web/static/src/js/fields/relational_fields.js +++ b/addons/web/static/src/js/fields/relational_fields.js @@ -15,7 +15,6 @@ odoo.define('web.relational_fields', function (require) { var AbstractField = require('web.AbstractField'); var concurrency = require('web.concurrency'); -var config = require('web.config'); var ControlPanel = require('web.ControlPanel'); var dialogs = require('web.view_dialogs'); var core = require('web.core'); diff --git a/addons/web/static/src/js/views/basic/basic_renderer.js b/addons/web/static/src/js/views/basic/basic_renderer.js index ed9726478a0..e150b0b2dd0 100644 --- a/addons/web/static/src/js/views/basic/basic_renderer.js +++ b/addons/web/static/src/js/views/basic/basic_renderer.js @@ -147,32 +147,45 @@ var BasicRenderer = AbstractRenderer.extend({ }, /** * Activates the widget at the given index for the given record if possible - * or the "next" possible one. + * or the "next" possible one. Usually, a widget can be activated if it is + * in edit mode, and if it is visible. * * @private * @param {Object} record * @param {integer} currentIndex - * @param {integer} [inc=1] - the increment to use when searching for the - * "next" possible one + * @param {Object} [options] + * @param {integer} [options.inc=1] - the increment to use when searching for the + * "next" possible one + * @param {boolean} [options.wrap=true] if true, when we arrive at the end of the + * list of widget, we wrap around and try to activate widgets starting at + * the beginning. Otherwise, we just stop trying and return -1 * @returns {integer} the index of the widget that was activated or -1 if - * none was possible to activate + * none was possible to activate */ - _activateFieldWidget: function (record, currentIndex, inc) { - inc = inc === undefined ? 1 : inc; + _activateFieldWidget: function (record, currentIndex, options) { + options = options || {}; + _.defaults(options, {inc: 1, wrap: true}); - var activated; var recordWidgets = this.allFieldWidgets[record.id] || []; for (var i = 0 ; i < recordWidgets.length ; i++) { - activated = recordWidgets[currentIndex].activate(); + var activated = recordWidgets[currentIndex].activate(); if (activated) { return currentIndex; } - currentIndex += inc; + currentIndex += options.inc; if (currentIndex >= recordWidgets.length) { - currentIndex -= recordWidgets.length; + if (options.wrap) { + currentIndex -= recordWidgets.length; + } else { + return -1; + } } else if (currentIndex < 0) { - currentIndex += recordWidgets.length; + if (options.wrap) { + currentIndex += recordWidgets.length; + } else { + return -1; + } } } return -1; @@ -188,7 +201,7 @@ var BasicRenderer = AbstractRenderer.extend({ */ _activateNextFieldWidget: function (record, currentIndex) { currentIndex = (currentIndex + 1) % (this.allFieldWidgets[record.id] || []).length; - return this._activateFieldWidget(record, currentIndex, +1); + return this._activateFieldWidget(record, currentIndex, {inc: 1}); }, /** * This is a wrapper of the {@see _activateFieldWidget} function to select @@ -201,7 +214,7 @@ var BasicRenderer = AbstractRenderer.extend({ */ _activatePreviousFieldWidget: function (record, currentIndex) { currentIndex = currentIndex ? (currentIndex - 1) : ((this.allFieldWidgets[record.id] || []).length - 1); - return this._activateFieldWidget(record, currentIndex, -1); + return this._activateFieldWidget(record, currentIndex, {inc:-1}); }, /** * Does the necessary DOM updates to match the given modifiers data. The diff --git a/addons/web/static/src/js/views/list/list_editable_renderer.js b/addons/web/static/src/js/views/list/list_editable_renderer.js index 12ef168c131..3d34b80f846 100644 --- a/addons/web/static/src/js/views/list/list_editable_renderer.js +++ b/addons/web/static/src/js/views/list/list_editable_renderer.js @@ -335,22 +335,27 @@ ListRenderer.include({ * * @param {integer} rowIndex * @param {integer} colIndex + * @param {boolean} [wrap=true] if true and no widget could be selected from + * the colIndex to the last column, then we wrap around and try to select + * a widget starting from the beginning + * @return {Deferred} fails if no cell could be selected */ - _selectCell: function (rowIndex, colIndex) { + _selectCell: function (rowIndex, colIndex, wrap) { // Do nothing if the user tries to select current cell if (rowIndex === this.currentRow && colIndex === this.currentCol) { - return; + return $.when(); } + wrap = wrap === undefined ? true : wrap; // Select the row then activate the widget in the correct cell var self = this; - this._selectRow(rowIndex).then(function () { + return this._selectRow(rowIndex).then(function () { var record = self.state.data[rowIndex]; - var fieldIndex = self._activateFieldWidget(record, colIndex - getNbButtonBefore(colIndex)); + var correctedIndex = colIndex - getNbButtonBefore(colIndex); + var fieldIndex = self._activateFieldWidget(record, correctedIndex, {inc: 1, wrap: wrap}); - // If no widget to activate in the row, unselect the row if (fieldIndex < 0) { - return self._unselectRow(); + return $.Deferred().reject(); } self.currentCol = fieldIndex + getNbButtonBefore(fieldIndex); @@ -543,7 +548,8 @@ ListRenderer.include({ // component up there, such as a form renderer. ev.stopPropagation(); if (this.currentCol + 1 < this.columns.length) { - this._selectCell(this.currentRow, this.currentCol + 1); + this._selectCell(this.currentRow, this.currentCol + 1, false) + .fail(this._moveToNextLine.bind(this)); } else { this._moveToNextLine(); } diff --git a/addons/web/static/tests/views/form_tests.js b/addons/web/static/tests/views/form_tests.js index 9295f0fa236..b6d416156aa 100644 --- a/addons/web/static/tests/views/form_tests.js +++ b/addons/web/static/tests/views/form_tests.js @@ -3075,6 +3075,35 @@ QUnit.module('Views', { form.destroy(); }); + QUnit.test('skip invisible fields when navigating with TAB', function (assert) { + assert.expect(1); + + this.data.partner.records[0].bar = true; + + var form = createView({ + View: FormView, + model: 'partner', + data: this.data, + arch: '
' + + '' + + '' + + '' + + '' + + '' + + '
', + res_id: 1, + }); + + form.$buttons.find('.o_form_button_edit').click(); + form.$('input[name="foo"]').focus(); + form.$('input[name="foo"]').trigger($.Event('keydown', {which: $.ui.keyCode.TAB})); + assert.strictEqual(form.$('input[name="int_field"]')[0], document.activeElement, + "int_field should be focused"); + + form.destroy(); + }); + QUnit.test('clicking on a stat button with a context', function (assert) { assert.expect(1); diff --git a/addons/web/static/tests/views/list_tests.js b/addons/web/static/tests/views/list_tests.js index ddfb985c3c1..50d5fbd7182 100644 --- a/addons/web/static/tests/views/list_tests.js +++ b/addons/web/static/tests/views/list_tests.js @@ -1784,6 +1784,112 @@ QUnit.module('Views', { list.destroy(); }); + QUnit.test('navigation with tab and readonly field', function (assert) { + // This test makes sure that if we have 2 cells in a row, the first in + // edit mode, and the second one readonly, then if we press TAB when the + // focus is on the first, then the focus skip the readonly cells and + // directly goes to the next line instead. + assert.expect(2); + + var list = createView({ + View: ListView, + model: 'foo', + data: this.data, + arch: '', + }); + + // click on first td and press TAB + list.$('td:contains(yop)').last().click(); + list.$('tr.o_selected_row input[name="foo"]').trigger({type: 'keydown', which: $.ui.keyCode.TAB}); + + assert.ok(list.$('tr.o_data_row:eq(1)').hasClass('o_selected_row'), + "2nd row should be selected"); + + // we do it again. This was broken because the this.currentRow variable + // was not properly set, and the second TAB could cause a crash. + list.$('tr.o_selected_row input[name="foo"]').trigger({type: 'keydown', which: $.ui.keyCode.TAB}); + assert.ok(list.$('tr.o_data_row:eq(2)').hasClass('o_selected_row'), + "3rd row should be selected"); + + list.destroy(); + }); + + QUnit.test('edition, then navigation with tab (with a readonly field)', function (assert) { + // This test makes sure that if we have 2 cells in a row, the first in + // edit mode, and the second one readonly, then if we edit and press TAB, + // (before debounce), the save operation is properly done (before + // selecting the next row) + assert.expect(4); + + var list = createView({ + View: ListView, + model: 'foo', + data: this.data, + arch: '', + mockRPC: function (route, args) { + if (args.method) { + assert.step(args.method); + } + return this._super.apply(this, arguments); + }, + fieldDebounce: 1, + }); + + // click on first td and press TAB + list.$('td:contains(yop)').click(); + list.$('tr.o_selected_row input[name="foo"]').val('new value').trigger('input'); + list.$('tr.o_selected_row input[name="foo"]').trigger({type: 'keydown', which: $.ui.keyCode.TAB}); + + assert.strictEqual(list.$('tbody tr:first td:contains(new value)').length, 1, + "should have the new value visible in dom"); + assert.verifySteps(["write", "read"]); + list.destroy(); + }); + + QUnit.test('skip invisible fields when navigating list view with TAB', function (assert) { + assert.expect(2); + + var list = createView({ + View: ListView, + model: 'foo', + data: this.data, + arch: '' + + '' + + '' + + '' + + '', + res_id: 1, + }); + + list.$('td:contains(gnap)').click(); + assert.strictEqual(list.$('input[name="foo"]')[0], document.activeElement, + "foo should be focused"); + list.$('input[name="foo"]').trigger($.Event('keydown', {which: $.ui.keyCode.TAB})); + assert.strictEqual(list.$('input[name="int_field"]')[0], document.activeElement, + "int_field should be focused"); + + list.destroy(); + }); + + QUnit.test('navigation: moving down with keydown', function (assert) { + assert.expect(2); + + var list = createView({ + View: ListView, + model: 'foo', + data: this.data, + arch: '', + }); + + list.$('td:contains(yop)').click(); + assert.ok(list.$('tr.o_data_row:eq(0)').hasClass('o_selected_row'), + "1st row should be selected"); + list.$('tr.o_selected_row input[name="foo"]').trigger({type: 'keydown', which: $.ui.keyCode.DOWN}); + assert.ok(list.$('tr.o_data_row:eq(1)').hasClass('o_selected_row'), + "2nd row should be selected"); + list.destroy(); + }); + }); });