[IMP] web: many2many editable list with X remove icon (#20978)

In many2many lists, the remove record from the widget is a trash bin.
It is confusing, as people feel they are deleting it when they are
only unlinking it.

So in many2many, we propose to change the trash icon by a 'X'.

This icon appears in many2many relations and in many2many widgets:
even though hr.expense.sheet defines the expense lines as a one2many,
it uses a many2many widget, so we use a 'X' instead of the trash bin.

To avoid ambiguity, we decided to rename 'delete' to 'remove'.
This commit is contained in:
Alexandre Kühn
2018-01-03 11:42:39 +01:00
committed by GitHub
parent c8fa2d581c
commit 8bc7cebd9c
5 changed files with 82 additions and 66 deletions
@@ -635,11 +635,11 @@ var FieldX2Many = AbstractField.extend({
discard_changes: '_onDiscardChanges',
edit_line: '_onEditLine',
field_changed: '_onFieldChanged',
kanban_record_delete: '_onDeleteRecord',
list_record_delete: '_onDeleteRecord',
open_record: '_onOpenRecord',
save_line: '_onSaveLine',
kanban_record_delete: '_onRemoveRecord',
list_record_remove: '_onRemoveRecord',
resequence: '_onResequence',
save_line: '_onSaveLine',
toggle_column_order: '_onToggleColumnOrder',
}),
@@ -661,6 +661,7 @@ var FieldX2Many = AbstractField.extend({
this.operations = [];
this.isReadonly = this.mode === 'readonly';
this.view = this.attrs.views[this.attrs.mode];
this.isMany2Many = this.field.type === 'many2many' || this.attrs.widget === 'many2many';
this.activeActions = {};
this.recordParams = {fieldName: this.name, viewType: this.viewType};
var arch = this.view && this.view.arch;
@@ -798,6 +799,7 @@ var FieldX2Many = AbstractField.extend({
mode: this.mode,
addCreateLine: !this.isReadonly && this.activeActions.create,
addTrashIcon: !this.isReadonly && this.activeActions.delete,
isMany2Many: this.isMany2Many,
viewType: viewType,
columnInvisibleFields: this.currentColInvisibleFields,
});
@@ -947,10 +949,9 @@ var FieldX2Many = AbstractField.extend({
* @private
* @param {OdooEvent} ev
*/
_onDeleteRecord: function (ev) {
_onRemoveRecord: function (ev) {
ev.stopPropagation();
var shouldForget = this.attrs.widget === 'many2many' || this.field.type === 'many2many';
var operation = shouldForget ? 'FORGET' : 'DELETE';
var operation = this.isMany2Many ? 'FORGET' : 'DELETE';
this._setValue({
operation: operation,
ids: [ev.data.id],
@@ -23,11 +23,11 @@ ListRenderer.include({
navigation_move: '_onNavigationMove',
}),
events: _.extend({}, ListRenderer.prototype.events, {
'click .o_field_x2many_list_row_add a': '_onAddRecord',
'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',
'click .o_field_x2many_list_row_add a': '_onAddRecord',
'click tr .o_list_record_remove': '_onRemoveIconClick',
}),
/**
* @override
@@ -46,6 +46,10 @@ ListRenderer.include({
// of each line, so the user can delete a record.
this.addTrashIcon = params.addTrashIcon;
// replace the trash icon by X in case of many2many relations
// so that it means 'unlink' instead of 'remove'
this.isMany2Many = params.isMany2Many;
this.currentRow = null;
this.currentFieldIndex = null;
},
@@ -489,6 +493,7 @@ ListRenderer.include({
/**
* Editable rows are possibly extended with a trash icon on their right, to
* allow deleting the corresponding record.
* For many2many editable lists, the trash bin is replaced by X.
*
* @override
* @param {any} record
@@ -498,8 +503,10 @@ ListRenderer.include({
_renderRow: function (record, index) {
var $row = this._super.apply(this, arguments);
if (this.addTrashIcon) {
var $icon = $('<span>', {class: 'fa fa-trash-o', name: 'delete'});
var $td = $('<td>', {class: 'o_list_record_delete'}).append($icon);
var $icon = this.isMany2Many ?
$('<i>', {class: 'fa fa-times', name: 'unlink'}) :
$('<i>', {class: 'fa fa-trash-o', name: 'delete'});
var $td = $('<td>', {class: 'o_list_record_remove'}).append($icon);
$row.append($td);
}
return $row;
@@ -776,6 +783,17 @@ ListRenderer.include({
break;
}
},
/**
* Triggers a remove event. I don't know why we stop the propagation of the
* event.
*
* @param {MouseEvent} event
*/
_onRemoveIconClick: function (event) {
event.stopPropagation();
var id = $(event.target).closest('tr').data('id');
this.trigger_up('list_record_remove', {id: id});
},
/**
* If the list view editable, just let the event bubble. We don't want to
* open the record in this case anyway.
@@ -799,17 +817,6 @@ ListRenderer.include({
this._super.apply(this, arguments);
}
},
/**
* Triggers a delete event. I don't know why we stop the propagation of the
* event.
*
* @param {MouseEvent} event
*/
_onTrashIconClick: function (event) {
event.stopPropagation();
var id = $(event.target).closest('tr').data('id');
this.trigger_up('list_record_delete', {id: id});
},
/**
* When a click happens outside the list view, or outside a currently
* selected row, we want to unselect it.
+1 -1
View File
@@ -66,7 +66,7 @@
}
}
.o_list_record_selector, .o_list_record_delete, .o_handle_cell {
.o_list_record_selector, .o_list_record_remove, .o_handle_cell {
width: 1px; // to prevent the column to expand
}
@@ -2040,8 +2040,8 @@ QUnit.module('relational_fields', {
"embedded one2many should not have a selector");
assert.ok(!form.$('.o_field_x2many_list_row_add').length,
"embedded one2many should not be editable");
assert.ok(!form.$('td.o_list_record_delete').length,
"embedded one2many records should not have a trash icon");
assert.ok(!form.$('td.o_list_record_remove').length,
"embedded one2many records should not have a remove icon");
form.$buttons.find('.o_form_button_edit').click();
@@ -2049,10 +2049,10 @@ QUnit.module('relational_fields', {
"embedded one2many should now be editable");
assert.strictEqual(form.$('.o_field_x2many_list_row_add').attr('colspan'), "2",
"should have colspan 2 (one for field foo, one for being below trash icon)");
"should have colspan 2 (one for field foo, one for being below remove icon)");
assert.ok(form.$('td.o_list_record_delete').length,
"embedded one2many records should have a trash icon");
assert.ok(form.$('td.o_list_record_remove').length,
"embedded one2many records should have a remove icon");
form.destroy();
});
@@ -2914,7 +2914,7 @@ QUnit.module('relational_fields', {
});
QUnit.test('one2many list: unlink one record', function (assert) {
assert.expect(5);
assert.expect(6);
this.data.partner.records[0].p = [2, 4];
var form = createView({
View: FormView,
@@ -2943,13 +2943,16 @@ QUnit.module('relational_fields', {
});
form.$buttons.find('.o_form_button_edit').click();
assert.strictEqual(form.$('td.o_list_record_delete span').length, 2,
"should have 2 delete buttons");
assert.strictEqual(form.$('td.o_list_record_remove i').length, 2,
"should have 2 remove buttons");
form.$('td.o_list_record_delete span').first().click();
assert.ok(form.$('td.o_list_record_remove i').first().hasClass('fa fa-times'),
"should have X icons to remove (unlink) records");
assert.strictEqual(form.$('td.o_list_record_delete span').length, 1,
"should have 1 delete button (a record is supposed to have been unlinked)");
form.$('td.o_list_record_remove i').first().click();
assert.strictEqual(form.$('td.o_list_record_remove i').length, 1,
"should have 1 remove button (a record is supposed to have been unlinked)");
// save and check that the correct command has been generated
form.$buttons.find('.o_form_button_save').click();
@@ -2957,7 +2960,7 @@ QUnit.module('relational_fields', {
});
QUnit.test('one2many list: deleting one record', function (assert) {
assert.expect(5);
assert.expect(6);
this.data.partner.records[0].p = [2, 4];
var form = createView({
View: FormView,
@@ -2986,13 +2989,16 @@ QUnit.module('relational_fields', {
});
form.$buttons.find('.o_form_button_edit').click();
assert.strictEqual(form.$('td.o_list_record_delete span').length, 2,
"should have 2 delete buttons");
assert.strictEqual(form.$('td.o_list_record_remove i').length, 2,
"should have 2 remove buttons");
form.$('td.o_list_record_delete span').first().click();
assert.ok(form.$('td.o_list_record_remove i').first().hasClass('fa fa-trash-o'),
"should have trash bin icons to remove (delete) records");
assert.strictEqual(form.$('td.o_list_record_delete span').length, 1,
"should have 1 delete button (a record is supposed to have been deleted)");
form.$('td.o_list_record_remove i').first().click();
assert.strictEqual(form.$('td.o_list_record_remove i').length, 1,
"should have 1 remove button (a record is supposed to have been deleted)");
// save and check that the correct command has been generated
form.$buttons.find('.o_form_button_save').click();
@@ -3164,8 +3170,8 @@ QUnit.module('relational_fields', {
},
});
assert.ok(!form.$('.o_list_record_delete').length,
'delete icon should not be visible in readonly');
assert.ok(!form.$('.o_list_record_remove').length,
'remove icon should not be visible in readonly');
assert.ok(!form.$('.o_field_x2many_list_row_add').length,
'"Add an item" should not be visible in readonly');
@@ -3175,8 +3181,8 @@ QUnit.module('relational_fields', {
'should contain 2 records');
assert.strictEqual(form.$('.o_list_view tbody td:first()').text(), 'second record',
'display_name of first subrecord should be the one in DB');
assert.ok(form.$('.o_list_record_delete').length,
'delete icon should be visible in edit');
assert.ok(form.$('.o_list_record_remove').length,
'remove icon should be visible in edit');
assert.ok(form.$('.o_field_x2many_list_row_add').length,
'"Add an item" should not visible in edit');
@@ -3192,8 +3198,8 @@ QUnit.module('relational_fields', {
// create new subrecords
// TODO when 'Add an item' will be implemented
// delete subrecords
form.$('.o_list_record_delete:nth(1)').click();
// remove subrecords
form.$('.o_list_record_remove:nth(1)').click();
assert.strictEqual(form.$('.o_list_view td.o_list_number').length, 1,
'should contain 1 subrecord');
assert.strictEqual(form.$('.o_list_view tbody td:first()').text(), 'new name',
@@ -3738,7 +3744,7 @@ QUnit.module('relational_fields', {
form.$('.o_data_cell:first').click();
form.$('.o_field_widget[name="product_id"] input').val('').trigger('keyup');
assert.verifySteps(['read', 'read'], 'no onchange should be done as line is invalid');
form.$('.o_list_record_delete').click();
form.$('.o_list_record_remove').click();
assert.verifySteps(['read', 'read', 'onchange'], 'onchange should have been done');
form.destroy();
@@ -5334,8 +5340,8 @@ QUnit.module('relational_fields', {
});
assert.strictEqual(form.$('.o_data_row').length, 2,
'there should be only 2 rows displayed');
form.$('.o_list_record_delete').click();
form.$('.o_list_record_delete').click();
form.$('.o_list_record_remove').click();
form.$('.o_list_record_remove').click();
assert.strictEqual(form.$('.o_data_row').length, 1,
'there should be just one remaining row');
@@ -5450,9 +5456,9 @@ QUnit.module('relational_fields', {
// disable the many2many onchange
form.$('input.o_field_integer[name="int_field"]').val('0').trigger('input');
// delete and start over
form.$('.o_list_record_delete:first span').click();
form.$('.o_list_record_delete:first span').click();
// remove and start over
form.$('.o_list_record_remove:first i').click();
form.$('.o_list_record_remove:first i').click();
// enable the many2many onchange
form.$('input.o_field_integer[name="int_field"]').val('10').trigger('input');
@@ -7256,7 +7262,7 @@ QUnit.module('relational_fields', {
},
});
assert.ok(!form.$('.o_list_record_delete').length,
assert.ok(!form.$('.o_list_record_remove').length,
'delete icon should not be visible in readonly');
assert.ok(!form.$('.o_field_x2many_list_row_add').length,
'"Add an item" should not be visible in readonly');
@@ -7267,7 +7273,7 @@ QUnit.module('relational_fields', {
'should contain 2 records');
assert.strictEqual(form.$('.o_list_view tbody td:first()').text(), 'gold',
'display_name of first subrecord should be the one in DB');
assert.ok(form.$('.o_list_record_delete').length,
assert.ok(form.$('.o_list_record_remove').length,
'delete icon should be visible in edit');
assert.ok(form.$('.o_field_x2many_list_row_add').length,
'"Add an item" should be visible in edit');
@@ -7292,8 +7298,8 @@ QUnit.module('relational_fields', {
assert.strictEqual(form.$('.o_list_view td.o_list_number').length, 3,
'should contain 3 subrecords');
// delete subrecords
form.$('.o_list_record_delete:nth(1)').click();
// remove subrecords
form.$('.o_list_record_remove:nth(1)').click();
assert.strictEqual(form.$('.o_list_view td.o_list_number').length, 2,
'should contain 2 subrecords');
assert.strictEqual(form.$('.o_list_view .o_data_row td:first').text(), 'new name',
@@ -7323,7 +7329,7 @@ QUnit.module('relational_fields', {
});
QUnit.test('many2many list (editable): edition', function (assert) {
assert.expect(30);
assert.expect(31);
this.data.partner.records[0].timmy = [12, 14];
this.data.partner_type.records.push({id: 15, display_name: "bronze", color: 6});
@@ -7358,7 +7364,7 @@ QUnit.module('relational_fields', {
res_id: 1,
});
assert.ok(!form.$('.o_list_record_delete').length,
assert.ok(!form.$('.o_list_record_remove').length,
'delete icon should not be visible in readonly');
assert.ok(!form.$('.o_field_x2many_list_row_add').length,
'"Add an item" should not be visible in readonly');
@@ -7369,8 +7375,10 @@ QUnit.module('relational_fields', {
'should contain 2 records');
assert.strictEqual(form.$('.o_list_view tbody td:first()').text(), 'gold',
'display_name of first subrecord should be the one in DB');
assert.ok(form.$('.o_list_record_delete').length,
assert.ok(form.$('.o_list_record_remove').length,
'delete icon should be visible in edit');
assert.ok(form.$('td.o_list_record_remove i').first().hasClass('fa fa-times'),
"should have X icons to remove (unlink) records");
assert.ok(form.$('.o_field_x2many_list_row_add').length,
'"Add an item" should not visible in edit');
@@ -7404,8 +7412,8 @@ QUnit.module('relational_fields', {
assert.strictEqual(form.$('.o_list_view td.o_list_number').length, 3,
'should contain 3 subrecords');
// delete subrecords
form.$('.o_list_record_delete:nth(1)').click();
// remove subrecords
form.$('.o_list_record_remove:nth(1)').click();
assert.strictEqual(form.$('.o_list_view td.o_list_number').length, 2,
'should contain 2 subrecord');
assert.strictEqual(form.$('.o_list_view tbody .o_data_row td:first').text(),
@@ -7453,7 +7461,7 @@ QUnit.module('relational_fields', {
form.$buttons.find('.o_form_button_edit').click();
assert.strictEqual(form.$('.o_field_x2many_list_row_add').length, 1, "should have the 'Add an item' link");
assert.strictEqual(form.$('.o_list_record_delete').length, 2, "should have the 'Add an item' link");
assert.strictEqual(form.$('.o_list_record_remove').length, 2, "should have the 'Add an item' link");
form.destroy();
@@ -7474,7 +7482,7 @@ QUnit.module('relational_fields', {
form.$buttons.find('.o_form_button_edit').click();
assert.strictEqual(form.$('.o_field_x2many_list_row_add').length, 0, "should not have the 'Add an item' link");
assert.strictEqual(form.$('.o_list_record_delete').length, 0, "should not have the 'Add an item' link");
assert.strictEqual(form.$('.o_list_record_remove').length, 0, "should not have the 'Add an item' link");
form.destroy();
});
@@ -246,11 +246,11 @@ odoo.define('web.test.x2many', function (require) {
run: function () {}, // don't change texarea content
}, { // remove
content: "remove b",
trigger: '.tab-pane:eq(0) .o_field_widget tbody tr:has(td:containsExact(bbb)) .o_list_record_delete',
trigger: '.tab-pane:eq(0) .o_field_widget tbody tr:has(td:containsExact(bbb)) .o_list_record_remove',
extra_trigger: '.tab-pane:eq(0) .o_field_widget tbody tr td:containsExact(aaa)',
}, {
content: "remove e",
trigger: 'tr:has(td:containsExact(e)) .o_list_record_delete',
trigger: 'tr:has(td:containsExact(e)) .o_list_record_remove',
extra_trigger: 'body:not(:has(tr:has(td:containsExact(bbb))))',
}, { // save
content: "save discussion",
@@ -258,7 +258,7 @@ odoo.define('web.test.x2many', function (require) {
extra_trigger: 'body:not(:has(tr:has(td:containsExact(e))))',
}, { // check saved data
content: "check data 4",
trigger: '.o_content:not(:has(.tab-pane:eq(0) .o_field_widget tbody tr:has(.o_list_record_delete):eq(4)))',
trigger: '.o_content:not(:has(.tab-pane:eq(0) .o_field_widget tbody tr:has(.o_list_record_remove):eq(4)))',
run: function () {}, // it's a check
}, {
content: "check data 5",
@@ -408,7 +408,7 @@ odoo.define('web.test.x2many', function (require) {
trigger: '.ui-autocomplete a:first',
}, { // remove record
content: "delete the last item in the editable list",
trigger: '.o_list_view .o_data_row td.o_list_record_delete span:visible:last',
trigger: '.o_list_view .o_data_row td.o_list_record_remove i:visible:last',
}, {
content: "test one2many onchange after delete",
trigger: '.o_content:not(:has(textarea[name="message_concat"]:propValueContains(Administrator:d)))',