From 59b55e22218d395083f48db58eaa322919f4fbc9 Mon Sep 17 00:00:00 2001
From: Khoi Nguyen <bng@odoo.com>
Date: Mon, 2 Oct 2017 15:59:20 +0200
Subject: [PATCH] [FIX] web: fix TAB key in editable list views
Before this commit, two variables were used to store the current
position in a list view: currentCol and currentRow. This made it
difficult to handle incremental moves (eg: tab, shift+tab) inside a
row, since we have to ignore all buttons. Previously, this was done by
converting 'currentCol' to 'currentFieldIndex', but the conversion
between the two variables was incorrect and does not appear to be
that easy to fix. As an example, it was not possible to TAB-navigate to
the last column of the components list when creating a BoM.
This commit removes all mentions of 'currentCol' and uses
'currentFieldIndex' instead, removing the need for conversions. The TAB
navigation code is simpler as a result, and should now be correct.
---
.../js/views/list/list_editable_renderer.js | 54 +++++++------------
addons/web/static/tests/views/list_tests.js | 4 +-
2 files changed, 22 insertions(+), 36 deletions(-)
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 8f0e73cb466c..9027fc3c9848 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
@@ -47,7 +47,6 @@ ListRenderer.include({
this.addTrashIcon = params.addTrashIcon;
this.currentRow = null;
- this.currentCol = null;
this.currentFieldIndex = null;
},
/**
@@ -175,7 +174,7 @@ ListRenderer.include({
});
if (self.currentRow !== null) {
self.currentRow = newRowIndex;
- return self._selectCell(newRowIndex, self.currentCol, {force: true}).then(function () {
+ return self._selectCell(newRowIndex, self.currentFieldIndex, {force: true}).then(function () {
// restore the cursor position
currentRowID = self.state.data[newRowIndex].id;
currentWidget = self.allFieldWidgets[currentRowID][self.currentFieldIndex];
@@ -458,7 +457,6 @@ ListRenderer.include({
*/
_render: function () {
this.currentRow = null;
- this.currentCol = null;
this.currentFieldIndex = null;
return this._super.apply(this, arguments);
},
@@ -560,22 +558,22 @@ ListRenderer.include({
* is, if necessary.
*
* @param {integer} rowIndex
- * @param {integer} colIndex
+ * @param {integer} fieldIndex
* @param {Object} [options]
* @param {Event} [options.event] original target of the event which
* @param {boolean} [options.wrap=true] if true and no widget could be
* triggered the cell selection
- * selected from the colIndex to the last column, then we wrap around and
+ * selected from the fieldIndex to the last column, then we wrap around and
* try to select a widget starting from the beginning
* @param {boolean} [options.force=false] if true, force selecting the cell
* even if seems to be already the selected one (useful after a re-
* rendering, to reset the focus on the correct field)
* @return {Deferred} fails if no cell could be selected
*/
- _selectCell: function (rowIndex, colIndex, options) {
+ _selectCell: function (rowIndex, fieldIndex, options) {
options = options || {};
// Do nothing if the user tries to select current cell
- if (!options.force && rowIndex === this.currentRow && colIndex === this.currentCol) {
+ if (!options.force && rowIndex === this.currentRow && fieldIndex === this.currentFieldIndex) {
return $.when();
}
var wrap = options.wrap === undefined ? true : options.wrap;
@@ -584,32 +582,18 @@ ListRenderer.include({
var self = this;
return this._selectRow(rowIndex).then(function () {
var record = self.state.data[rowIndex];
- var correctedIndex = colIndex - getNbButtonBefore(colIndex);
- if (correctedIndex >= (self.allFieldWidgets[record.id] || []).length) {
+ if (fieldIndex >= (self.allFieldWidgets[record.id] || []).length) {
return $.Deferred().reject();
}
- var fieldIndex = self._activateFieldWidget(record, correctedIndex, {
+ fieldIndex = self._activateFieldWidget(record, fieldIndex, {
inc: 1,
wrap: wrap,
event: options && options.event,
});
-
if (fieldIndex < 0) {
return $.Deferred().reject();
}
-
- self.currentCol = fieldIndex + getNbButtonBefore(fieldIndex);
self.currentFieldIndex = fieldIndex;
-
- function getNbButtonBefore(index) {
- var nbButtons = 0;
- for (var i = 0 ; i < index ; i++) {
- if (self.columns[i].tag === 'button') {
- nbButtons++;
- }
- }
- return nbButtons;
- }
});
},
/**
@@ -682,8 +666,8 @@ ListRenderer.include({
var $td = $(event.currentTarget);
var $tr = $td.parent();
var rowIndex = this.$('.o_data_row').index($tr);
- var colIndex = $tr.find('.o_data_cell').index($td);
- this._selectCell(rowIndex, colIndex, {event: event});
+ var fieldIndex = Math.max($tr.find('.o_data_cell').not('.o_list_button').index($td), 0);
+ this._selectCell(rowIndex, fieldIndex, {event: event});
},
/**
* We need to manually unselect row, because noone else would do it
@@ -725,35 +709,35 @@ ListRenderer.include({
switch (ev.data.direction) {
case 'up':
if (this.currentRow > 0) {
- this._selectCell(this.currentRow - 1, this.currentCol);
+ this._selectCell(this.currentRow - 1, this.currentFieldIndex);
}
break;
case 'right':
- if (this.currentCol + 1 < this.columns.length) {
- this._selectCell(this.currentRow, this.currentCol + 1);
+ if (this.currentFieldIndex + 1 < this.columns.length) {
+ this._selectCell(this.currentRow, this.currentFieldIndex + 1);
}
break;
case 'down':
if (this.currentRow < this.state.data.length - 1) {
- this._selectCell(this.currentRow + 1, this.currentCol);
+ this._selectCell(this.currentRow + 1, this.currentFieldIndex);
}
break;
case 'left':
- if (this.currentCol > 0) {
- this._selectCell(this.currentRow, this.currentCol - 1);
+ if (this.currentFieldIndex > 0) {
+ this._selectCell(this.currentRow, this.currentFieldIndex - 1);
}
break;
case 'previous':
- if (this.currentCol > 0) {
- this._selectCell(this.currentRow, this.currentCol - 1, {wrap: false})
+ if (this.currentFieldIndex > 0) {
+ this._selectCell(this.currentRow, this.currentFieldIndex - 1, {wrap: false})
.fail(this._moveToPreviousLine.bind(this));
} else {
this._moveToPreviousLine();
}
break;
case 'next':
- if (this.currentCol + 1 < this.columns.length) {
- this._selectCell(this.currentRow, this.currentCol + 1, {wrap: false})
+ if (this.currentFieldIndex + 1 < this.columns.length) {
+ this._selectCell(this.currentRow, this.currentFieldIndex + 1, {wrap: false})
.fail(this._moveToNextLine.bind(this));
} else {
this._moveToNextLine();
diff --git a/addons/web/static/tests/views/list_tests.js b/addons/web/static/tests/views/list_tests.js
index 52149f008ebb..4a89a4a70e70 100644
--- a/addons/web/static/tests/views/list_tests.js
+++ b/addons/web/static/tests/views/list_tests.js
@@ -2390,6 +2390,8 @@ QUnit.module('Views', {
model: 'foo',
data: this.data,
arch: '<tree editable="bottom">' +
+ // Adding a button column makes conversions between column and field position trickier
+ '<button name="kikou" string="Kikou" type="object"/>' +
'<field name="foo"/>' +
'<button name="kikou" string="Kikou" type="object"/>' +
'<field name="int_field"/>' +
@@ -2397,7 +2399,7 @@ QUnit.module('Views', {
res_id: 1,
});
- list.$('tbody tr:eq(2) td:eq(1)').click();
+ list.$('tbody tr:eq(2) td:eq(2)').click();
assert.strictEqual(list.$('tbody tr:eq(2) input[name="foo"]')[0], document.activeElement,
"foo should be focused");
list.$('tbody tr:eq(2) input[name="foo"]').trigger($.Event('keydown', {which: $.ui.keyCode.TAB}));
--
GitLab