From 2c59d81d44aa408f980fd08bef0a39eacd2e52d1 Mon Sep 17 00:00:00 2001 From: qsm-odoo Date: Tue, 2 May 2017 10:38:47 +0200 Subject: [PATCH] [FIX] web: edit correct list line on click after line deletion Before this commit, if the user created a line (thanks to the create button) then deleted it, the rows were in a broken state which made the list edit the wrong line on line click (it opened the next one). This was due to row indexes which are kept as data on rows: they were not updated on row deletion. This commit simplifies the code by getting rid of this indexes data and instead compute those when necessary. --- .../js/views/list/list_editable_renderer.js | 64 +++++-------------- .../static/src/js/views/list/list_renderer.js | 23 ++++--- addons/web/static/tests/views/list_tests.js | 28 ++++++++ 3 files changed, 57 insertions(+), 58 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 f9468418b9b..a9b9cacd9c9 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 @@ -25,7 +25,7 @@ ListRenderer.include({ move_next_line: '_onMoveNextLine', }), events: _.extend({}, ListRenderer.prototype.events, { - 'click tbody td': '_onCellClick', + 'click tbody td.o_data_cell': '_onCellClick', 'click tbody tr:not(.o_data_row)': '_onEmptyRowClick', 'click tfoot': '_onFooterClick', 'click tr .o_list_record_delete': '_onTrashIconClick', @@ -161,8 +161,8 @@ ListRenderer.include({ if (rowIndex === this.currentRow) { this.currentRow = null; } - var $row = this.$('tbody > tr:nth(' + rowIndex + ')'); - $row.remove(); // FIXME row indexes are not updated... let's remove data('index') and check index dynamically ? + var $row = this.$('.o_data_row:nth(' + rowIndex + ')'); + $row.remove(); this.state = state; }, @@ -184,14 +184,15 @@ ListRenderer.include({ var record = this.state.data[rowIndex]; this.currentRow = editMode ? rowIndex : null; - var $row = this.$('tbody > tr:nth(' + rowIndex + ')'); + var $row = this.$('.o_data_row:nth(' + rowIndex + ')'); $row.toggleClass('o_selected_row', editMode); if (editMode) { // Instantiate column widgets and destroy potential readonly ones var oldWidgets = _.clone(this.allFieldWidgets[record.id]); + var $tds = $row.children('.o_data_cell'); _.each(this.columns, function (node, colIndex) { - var $td = self._findTd($row, colIndex); + var $td = $tds.eq(colIndex); var $newTd = self._renderBodyCell(record, node, colIndex, { renderInvisible: true, renderWidgets: true, @@ -220,18 +221,6 @@ ListRenderer.include({ // Private //-------------------------------------------------------------------------- - /** - * Find the td in a row corresponding the a field with a given index. The - * main problem is that in some cases, we have a 'selector' checkbox, so the - * correct td is sometimes at position i, sometimes at i+1 - * - * @param {jQueryElement} $row the $tr for the row we are interested in - * @param {number} index the index of the field/column - * @returns {jQueryElement} the $td for the index-th field - */ - _findTd: function ($row, index) { - return $row.children(':nth(' + (index + (this.hasSelectors ? 1 : 0)) + ')'); - }, /** * Returns the current number of columns. The editable renderer may add a * trash icon on the right of a record, so we need to take this into account @@ -291,27 +280,8 @@ ListRenderer.include({ return $body; }, /** - * Add all the necessary styling classes to displayed cells. Also, we need - * to add the current index to each cell data attribute, so we can get it - * back when we need to select a cell. - * - * @override - * @param {Object} record - * @param {Object} node - * @param {integer} index - * @param {Object} [options] - * @returns {jQueryElement} - */ - _renderBodyCell: function (record, node, index, options) { - var $cell = this._super.apply(this, arguments); - if (this.mode === 'readonly') { - return $cell; - } - return $cell.data('index', index); - }, - /** - * Editable rows are extended with their index, and possibly a trash icon on - * their right, to allow deleting the corresponding record. + * Editable rows are possibly extended with a trash icon on their right, to + * allow deleting the corresponding record. * * @override * @param {any} record @@ -320,12 +290,9 @@ ListRenderer.include({ */ _renderRow: function (record, index) { var $row = this._super.apply(this, arguments); - if (this.mode === 'edit') { - $row.data('index', index); - } if (this.addTrashIcon) { - var $icon = $('').addClass('fa fa-trash-o').attr('name', 'delete'); - var $td = $('').addClass('o_list_record_delete').append($icon); + var $icon = $('', {class: 'fa fa-trash-o', name: 'delete'}); + var $td = $('', {class: 'o_list_record_delete'}).append($icon); $row.append($td); } return $row; @@ -432,7 +399,7 @@ ListRenderer.include({ var record = this.state.data[rowIndex]; var node = this.columns[colIndex]; - var $oldTd = this._findTd(this.$('tbody > tr:nth(' + rowIndex + ')'), colIndex); + var $oldTd = this.$('.o_data_row:nth(' + rowIndex + ') > .o_data_cell:nth(' + colIndex + ')'); var $newTd = this._renderBodyCell(record, node, colIndex, {mode: 'readonly'}); if (keepDirtyFlag && $oldTd.hasClass('o_field_dirty')) { $newTd.addClass('o_field_dirty'); @@ -515,11 +482,10 @@ ListRenderer.include({ return; } var $td = $(event.currentTarget); - var rowIndex = $td.parent().data('index'); - var colIndex = $td.data('index'); - if (colIndex !== undefined && rowIndex !== undefined) { - this._selectCell(rowIndex, colIndex); - } + 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); }, /** * We need to manually unselect row, because noone else would do it diff --git a/addons/web/static/src/js/views/list/list_renderer.js b/addons/web/static/src/js/views/list/list_renderer.js index c788ca0d44b..2305530affe 100644 --- a/addons/web/static/src/js/views/list/list_renderer.js +++ b/addons/web/static/src/js/views/list/list_renderer.js @@ -260,15 +260,17 @@ var ListRenderer = BasicRenderer.extend({ * @returns {jQueryElement} a element */ _renderBodyCell: function (record, node, colIndex, options) { - var tdClassName; + var tdClassName = 'o_data_cell'; if (node.tag === 'button') { - tdClassName = 'o_list_button'; - } else if (node.attrs.widget){ - tdClassName = ('o_' + node.attrs.widget + '_cell'); + tdClassName += ' o_list_button'; } else { - tdClassName = FIELD_CLASSES[this.state.fields[node.attrs.name].type]; + if (node.attrs.widget){ + tdClassName += (' o_' + node.attrs.widget + '_cell'); + } else { + tdClassName += (' ' + FIELD_CLASSES[this.state.fields[node.attrs.name].type]); + } } - var $td = $('', { class: tdClassName }); + var $td = $('', {class: tdClassName}); // We register modifiers on the element so that it gets the correct // modifiers classes (for styling) @@ -550,14 +552,17 @@ var ListRenderer = BasicRenderer.extend({ * @returns {jQueryElement} a element */ _renderRow: function (record) { - var decorations = this._computeDecorationClassNames(record); var self = this; var $cells = _.map(this.columns, function (node, index) { return self._renderBodyCell(record, node, index, {mode: 'readonly'}); }); - var $tr = $('') + var className = 'o_data_row'; + var decorations = this._computeDecorationClassNames(record); + if (decorations.length) { + className += (' ' + decorations.join(' ')); + } + var $tr = $('', {class: className}) .data('id', record.id) - .addClass(decorations.length && decorations.join(' ')) .append($cells); if (this.hasSelectors) { $tr.prepend(this._renderSelector('td')); diff --git a/addons/web/static/tests/views/list_tests.js b/addons/web/static/tests/views/list_tests.js index 9ad2f10e631..4a8c87be598 100644 --- a/addons/web/static/tests/views/list_tests.js +++ b/addons/web/static/tests/views/list_tests.js @@ -1693,6 +1693,34 @@ QUnit.module('Views', { list.destroy(); }); + QUnit.test('edit list line after line deletion', function (assert) { + assert.expect(5); + + var list = createView({ + View: ListView, + model: 'foo', + data: this.data, + arch: '', + }); + + list.$('.o_data_row:nth(2) > td:not(.o_list_record_selector)').first().click(); + assert.ok(list.$('.o_data_row:nth(2)').is('.o_selected_row'), + "third row should be in edition"); + list.$buttons.find('.o_list_button_discard').click(); + list.$buttons.find('.o_list_button_add').click(); + assert.ok(list.$('.o_data_row:nth(0)').is('.o_selected_row'), + "first row should be in edition (creation)"); + list.$buttons.find('.o_list_button_discard').click(); + assert.strictEqual(list.$('.o_selected_row').length, 0, + "no row should be selected"); + list.$('.o_data_row:nth(2) > td:not(.o_list_record_selector)').first().click(); + assert.ok(list.$('.o_data_row:nth(2)').is('.o_selected_row'), + "third row should be in edition"); + assert.strictEqual(list.$('.o_selected_row').length, 1, + "no other row should be selected"); + + list.destroy(); + }); }); });