pengyuthon/odoo_source
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

[IMP] web: form: labels of several occurrences of a field

Browse Source 
Before this commit, when the same field occurred multiple times in
a form view, all occurrences were associated with the same id, and
all occurrences of the corresponding label (if any) had the same
'for' attribute. As a consequence, besides the fact that there were
warnings in the chrome console, clicking on any occurrence of the
duplicated label focuses the first occurrence of the field.

This commit generates a unique id for each pair label/field, when
possible. However, there is a case that can't be handled
automatically: when the label isn't automatically generated (field
located outside a group, or with nolabel attribute set to "1").
Typically, in this case, there is a <label> node with "for"
attribute referencing the associated field. In this situation, the
id must be defined in the arch, and used in the "for" attribute:

  <form>
    <label for="phone"/><field name="phone"/>
    <label for="phone_2"/><field name="phone" id="phone_2"/>
  </form>

Task 2261697
This commit is contained in:
Aaron Bohy
2020-07-02 13:57:37 +00:00
parent 25258e314e
commit e17dc1a793
6 changed files with 314 additions and 40 deletions
Download Patch File Download Diff File Expand all files Collapse all files
addons/web/static/src/js/views/form/form_renderer.js
+33 -29
View File
@@ -11,6 +11,9 @@ var viewUtils = require('web.viewUtils');
var _t = core._t;
var qweb = core.qweb;
// symbol used as key to set the <field> node id on its widget
const symbol = Symbol('form');
var FormRenderer = BasicRenderer.extend(WidgetAdapterMixin, {
className: "o_form_view",
events: _.extend({}, BasicRenderer.prototype.events, {
@@ -28,9 +31,12 @@ var FormRenderer = BasicRenderer.extend(WidgetAdapterMixin, {
/**
* @override
* @param {Object} params.fieldIdsToNames maps <field> node ids to field names
* (useful when there are several occurrences of the same field in the arch)
*/
init: function () {
init: function (parent, state, params) {
this._super.apply(this, arguments);
this.fieldIdsToNames = params.fieldIdsToNames;
this.idsForLabels = {};
this.lastActivatedFieldIndex = -1;
this.alertFields = {};
@@ -113,20 +119,21 @@ var FormRenderer = BasicRenderer.extend(WidgetAdapterMixin, {
* @returns {string[]}
*/
canBeSaved: function () {
var self = this;
var fieldNames = this._super.apply(this, arguments);
var $labels = this.$('label');
$labels.removeClass('o_field_invalid');
_.each(fieldNames, function (fieldName) {
var idForLabel = self.idsForLabels[fieldName];
const allWidgets = this.allFieldWidgets[this.state.id] || [];
const widgets = allWidgets.filter(w => fieldNames.includes(w.name));
for (const widget of widgets) {
const idForLabel = this.idsForLabels[widget[symbol]];
if (idForLabel) {
$labels
.filter('[for=' + idForLabel + ']')
.addClass('o_field_invalid');
}
});
}
return fieldNames;
},
/*
@@ -171,7 +178,7 @@ var FormRenderer = BasicRenderer.extend(WidgetAdapterMixin, {
var self = this;
return this._super.apply(this, arguments).then(function (resetWidgets) {
_.each(resetWidgets, function (widget) {
self._setIDForLabel(widget, self.idsForLabels[widget.name]);
self._setIDForLabel(widget, self.idsForLabels[widget[symbol]]);
});
if (self.$('.o_field_invalid').length) {
self.canBeSaved(self.state.id);
@@ -372,16 +379,14 @@ var FormRenderer = BasicRenderer.extend(WidgetAdapterMixin, {
},
/**
* @private
* @param {string} name
* @param {string} uid a <field> node id
* @returns {string}
*/
_getIDForLabel: function (name) {
var idForLabel = this.idsForLabels[name];
if (!idForLabel) {
idForLabel = _.uniqueId('o_field_input_');
this.idsForLabels[name] = idForLabel;
_getIDForLabel: function (uid) {
if (!this.idsForLabels[uid]) {
this.idsForLabels[uid] = _.uniqueId('o_field_input_');
}
return idForLabel;
return this.idsForLabels[uid];
},
/**
* @override
@@ -396,7 +401,9 @@ var FormRenderer = BasicRenderer.extend(WidgetAdapterMixin, {
*/
_postProcessField: function (widget, node) {
this._super.apply(this, arguments);
this._setIDForLabel(widget, this._getIDForLabel(node.attrs.name));
// set the node id on the widget, as it might be necessary later (tooltips, confirmChange...)
widget[symbol] = node.attrs.id;
this._setIDForLabel(widget, this._getIDForLabel(node.attrs.id));
if (JSON.parse(node.attrs.default_focus || "0")) {
this.defaultFocusField = widget;
}
@@ -656,12 +663,12 @@ var FormRenderer = BasicRenderer.extend(WidgetAdapterMixin, {
},
/**
* @private
* @param {string} label
* @param {Object} node
* @returns {jQueryElement}
*/
_renderInnerGroupLabel: function (label) {
_renderInnerGroupLabel: function (node) {
return $('<td/>', {class: 'o_td_label'})
.append(this._renderTagLabel(label));
.append(this._renderTagLabel(node));
},
/**
* Render a node, from the arch of the view. It is a generic method, that
@@ -846,17 +853,22 @@ var FormRenderer = BasicRenderer.extend(WidgetAdapterMixin, {
_renderTagLabel: function (node) {
var self = this;
var text;
var fieldName = node.tag === 'label' ? node.attrs.for : node.attrs.name;
let fieldName;
if (node.tag === 'label') {
fieldName = this.fieldIdsToNames[node.attrs.for]; // 'for' references a <field> node id
} else {
fieldName = node.attrs.name;
}
if ('string' in node.attrs) { // allow empty string
text = node.attrs.string;
} else if (fieldName) {
text = this.state.fields[fieldName].string;
} else {
} else {
return this._renderGenericTag(node);
}
var $result = $('<label>', {
class: 'o_form_label',
for: this._getIDForLabel(fieldName),
for: this._getIDForLabel(node.tag === 'label' ? node.attrs.for : node.attrs.id),
text: text,
});
if (node.tag === 'label') {
@@ -1039,16 +1051,8 @@ var FormRenderer = BasicRenderer.extend(WidgetAdapterMixin, {
// Attach the tooltips on the fields' label
_.each(this.allFieldWidgets[this.state.id], function (widget) {
var idForLabel = self.idsForLabels[widget.name];
// We usually don't support multiple widgets for the same field on the
// same view but it is the case with the new settings view on V11.0.
// Therefore, we need to retrieve the correct label since it could be
// displayed multiple times on the view, otherwise, for example the
// enterprise label will be displayed as many times as the field
// exists on settings.
var $widgets = self.$('.o_field_widget[name=' + widget.name + ']');
const idForLabel = self.idsForLabels[widget[symbol]];
var $label = idForLabel ? self.$('.o_form_label[for=' + idForLabel + ']') : $();
$label = $label.eq($widgets.index(widget.$el));
self._addFieldTooltip(widget, $label);
if (widget.attrs.widget === 'upgrade_boolean') {
// this widget needs a reference to its $label to be correctly
addons/web/static/src/js/views/form/form_view.js
+35 -1
View File
@@ -2,11 +2,11 @@ odoo.define('web.FormView', function (require) {
"use strict";
var BasicView = require('web.BasicView');
var config = require('web.config');
var Context = require('web.Context');
var core = require('web.core');
var FormController = require('web.FormController');
var FormRenderer = require('web.FormRenderer');
const { generateID } = require('web.utils');
var _lt = core._lt;
@@ -48,6 +48,7 @@ var FormView = BasicView.extend({
this.rendererParams.mode = mode;
this.rendererParams.isFromFormViewDialog = params.isFromFormViewDialog;
this.rendererParams.fieldIdsToNames = this.fieldsView.fieldIdsToNames;
},
//--------------------------------------------------------------------------
@@ -148,6 +149,39 @@ var FormView = BasicView.extend({
}
return Promise.all(defs);
},
/**
* @override
*/
_processArch(arch, fv) {
fv.fieldIdsToNames = {}; // maps field ids (identifying <field> nodes) to field names
return this._super(...arguments);
},
/**
* Override to populate the 'fieldIdsToNames' dict mapping <field> node ids
* to field names. Those ids are computed as follows:
* - if set on the node, we use the 'id' attribute
* - otherwise
* - if this is the first occurrence of the field in the arch, we use
* its name as id ('name' attribute)
* - otherwise we generate an id by concatenating the field name with
* a unique id
* - in both cases, we set the id we generated in the attrs, as it
* will be used by the renderer.
*
* @override
*/
_processNode(node, fv) {
if (node.tag === 'field') {
const name = node.attrs.name;
let uid = node.attrs.id;
if (!uid) {
uid = name in fv.fieldIdsToNames ? `${name}__${generateID()}__` : name;
node.attrs.id = uid;
}
fv.fieldIdsToNames[uid] = name;
}
return this._super(...arguments);
},
/**
* We set here the limit for the number of records fetched (in one page).
* This method is only called for subviews, not for main views.
addons/web/static/tests/views/form_tests.js
+210
View File
@@ -990,6 +990,216 @@ QUnit.module('Views', {
form.destroy();
});
QUnit.test('input ids for multiple occurrences of fields in form view', async function (assert) {
// A same field can occur several times in the view, but its id must be
// unique by occurrence, otherwise there is a warning in the console (in
// edit mode) as we get several inputs with the same "id" attribute, and
// several labels the same "for" attribute.
assert.expect(2);
var form = await createView({
View: FormView,
model: 'partner',
data: this.data,
arch: `
<form>
<group>
<field name="foo"/>
<label for="qux"/>
<div><field name="qux"/></div>
</group>
<group>
<field name="foo"/>
<label for="qux2"/>
<div><field name="qux" id="qux2"/></div>
</group>
</form>`,
});
const fieldIdAttrs = [...form.$('.o_field_widget')].map(n => n.getAttribute('id'));
const labelForAttrs = [...form.$('.o_form_label')].map(n => n.getAttribute('for'));
assert.strictEqual([...new Set(fieldIdAttrs)].length, 4,
"should have generated a unique id for each field occurrence");
assert.deepEqual(fieldIdAttrs, labelForAttrs,
"the for attribute of labels must coincide with field ids");
form.destroy();
});
QUnit.test('input ids for multiple occurrences of fields in sub form view (inline)', async function (assert) {
// A same field can occur several times in the view, but its id must be
// unique by occurrence, otherwise there is a warning in the console (in
// edit mode) as we get several inputs with the same "id" attribute, and
// several labels the same "for" attribute.
assert.expect(3);
var form = await createView({
View: FormView,
model: 'partner',
data: this.data,
arch: `
<form>
<field name="p">
<tree><field name="foo"/></tree>
<form>
<group>
<field name="foo"/>
<label for="qux"/>
<div><field name="qux"/></div>
</group>
<group>
<field name="foo"/>
<label for="qux2"/>
<div><field name="qux" id="qux2"/></div>
</group>
</form>
</field>
</form>`,
});
await testUtils.dom.click(form.$('.o_field_x2many_list_row_add a'));
assert.containsOnce(document.body, '.modal .o_form_view');
const fieldIdAttrs = [...$('.modal .o_form_view .o_field_widget')].map(n => n.getAttribute('id'));
const labelForAttrs = [...$('.modal .o_form_view .o_form_label')].map(n => n.getAttribute('for'));
assert.strictEqual([...new Set(fieldIdAttrs)].length, 4,
"should have generated a unique id for each field occurrence");
assert.deepEqual(fieldIdAttrs, labelForAttrs,
"the for attribute of labels must coincide with field ids");
form.destroy();
});
QUnit.test('input ids for multiple occurrences of fields in sub form view (not inline)', async function (assert) {
// A same field can occur several times in the view, but its id must be
// unique by occurrence, otherwise there is a warning in the console (in
// edit mode) as we get several inputs with the same "id" attribute, and
// several labels the same "for" attribute.
assert.expect(3);
var form = await createView({
View: FormView,
model: 'partner',
data: this.data,
arch: '<form><field name="p"/></form>',
archs: {
'partner,false,list': '<tree><field name="foo"/></tree>',
'partner,false,form': `
<form>
<group>
<field name="foo"/>
<label for="qux"/>
<div><field name="qux"/></div>
</group>
<group>
<field name="foo"/>
<label for="qux2"/>
<div><field name="qux" id="qux2"/></div>
</group>
</form>`
},
});
await testUtils.dom.click(form.$('.o_field_x2many_list_row_add a'));
assert.containsOnce(document.body, '.modal .o_form_view');
const fieldIdAttrs = [...$('.modal .o_form_view .o_field_widget')].map(n => n.getAttribute('id'));
const labelForAttrs = [...$('.modal .o_form_view .o_form_label')].map(n => n.getAttribute('for'));
assert.strictEqual([...new Set(fieldIdAttrs)].length, 4,
"should have generated a unique id for each field occurrence");
assert.deepEqual(fieldIdAttrs, labelForAttrs,
"the for attribute of labels must coincide with field ids");
form.destroy();
});
QUnit.test('two occurrences of invalid field in form view', async function (assert) {
assert.expect(2);
this.data.partner.fields.trululu.required = true;
var form = await createView({
View: FormView,
model: 'partner',
data: this.data,
arch: `
<form>
<group>
<field name="trululu"/>
<field name="trululu"/>
</group>
</form>`,
});
await testUtils.form.clickSave(form);
assert.containsN(form, '.o_form_label.o_field_invalid', 2);
assert.containsN(form, '.o_field_many2one.o_field_invalid', 2);
form.destroy();
});
QUnit.test('tooltips on multiple occurrences of fields and labels', async function (assert) {
assert.expect(4);
const initialDebugMode = odoo.debug;
odoo.debug = false;
this.data.partner.fields.foo.help = 'foo tooltip';
this.data.partner.fields.bar.help = 'bar tooltip';
const form = await createView({
View: FormView,
model: 'partner',
data: this.data,
arch: `
<form>
<group>
<field name="foo"/>
<label for="bar"/>
<div><field name="bar"/></div>
</group>
<group>
<field name="foo"/>
<label for="bar2"/>
<div><field name="bar" id="bar2"/></div>
</group>
</form>`,
});
const $fooLabel1 = form.$('.o_form_label:nth(0)');
$fooLabel1.tooltip('show', false);
$fooLabel1.trigger($.Event('mouseenter'));
assert.strictEqual($('.tooltip .oe_tooltip_help').text().trim(), "foo tooltip");
$fooLabel1.trigger($.Event('mouseleave'));
const $fooLabel2 = form.$('.o_form_label:nth(2)');
$fooLabel2.tooltip('show', false);
$fooLabel2.trigger($.Event('mouseenter'));
assert.strictEqual($('.tooltip .oe_tooltip_help').text().trim(), "foo tooltip");
$fooLabel2.trigger($.Event('mouseleave'));
const $barLabel1 = form.$('.o_form_label:nth(1)');
$barLabel1.tooltip('show', false);
$barLabel1.trigger($.Event('mouseenter'));
assert.strictEqual($('.tooltip .oe_tooltip_help').text().trim(), "bar tooltip");
$barLabel1.trigger($.Event('mouseleave'));
const $barLabel2 = form.$('.o_form_label:nth(3)');
$barLabel2.tooltip('show', false);
$barLabel2.trigger($.Event('mouseenter'));
assert.strictEqual($('.tooltip .oe_tooltip_help').text().trim(), "bar tooltip");
$barLabel2.trigger($.Event('mouseleave'));
odoo.debug = initialDebugMode;
form.destroy();
});
QUnit.test('readonly attrs on fields are re-evaluated on field change', async function (assert) {
assert.expect(4);
doc/reference/views.rst
+24
View File
@@ -800,6 +800,9 @@ system. Available semantic components are:
``name`` (mandatory)
the name of the field to render
``id``
the node id. Useful when there are several occurrences of the same field in
the view (see ``label`` component below). Default is the field name.
``widget``
fields have a default rendering based on their type
(e.g. :class:`~odoo.fields.Char`,
@@ -871,6 +874,27 @@ system. Available semantic components are:
for opening specific kanban view when selecting records from m2o/m2m in mobile
environment
``label``
when a ``field`` component isn't placed directly inside a ``group``, or when
its ``nolabel`` attribute is set, the field's label isn't automatically
displayed alongside its value. The ``label`` component is the manual alternative
of displaying the label of a field. Possible attributes are:
``for`` (mandatory)
the reference to the field associated with the label. Can be either the name
of a field, or its id (``id`` attribute set on the ``field``). When there are
several occurrences of the same field in the view, and there are several
``label`` components associated with these ``field`` nodes, those labels
must have unique ``for`` attributes (in this case referencing the ``id``
attribute of the corresponding ``field`` nodes).
``string``
the label to display. Display the field's label (coming from the field
definition in the model) by default.
``class``
same as for ``field`` component.
``attrs``
same as for ``field`` component.
Generic structure
~~~~~~~~~~~~~~~~~
odoo/addons/base/models/ir_ui_view.py
+11 -9
View File
@@ -969,7 +969,7 @@ actual arch.
def _postprocess_tag_field(self, node, name_manager, node_info):
if node.get('name'):
attrs = {'select': node.get('select')}
attrs = {'id': node.get('id'), 'select': node.get('select')}
field = name_manager.Model._fields.get(node.get('name'))
if field:
# apply groups (no tested)
@@ -1224,7 +1224,7 @@ actual arch.
'without corresponding field or button, use \'class="o_form_label"\'.')
self.handle_view_error(msg)
else:
name_manager.must_have_name(for_, 'label for') # this could be done in check_attr
name_manager.must_have_name_or_id(for_, 'label for') # this could be done in check_attr
def _validate_tag_page(self, node, name_manager, node_info):
if node.getparent() is None or node.getparent().tag != 'notebook':
@@ -1908,13 +1908,15 @@ class NameManager:
self.mandatory_fields = dict()
self.mandatory_parent_fields = dict()
self.available_actions = set()
self.mandatory_names = dict()
self.mandatory_names_or_ids = dict()
self.available_names_or_ids = set()
self.validate = validate
self.Model = Model
self.fields_get = self.Model.fields_get()
def has_field(self, name, info=()):
self.available_fields.setdefault(name, {}).update(info)
self.available_names_or_ids.add(info.get('id') or name)
def has_action(self, name):
self.available_actions.add(name)
@@ -1929,8 +1931,8 @@ class NameManager:
for name, use in name_uses.items():
self.must_have_field(name, use)
def must_have_name(self, name, use):
self.mandatory_names[name] = use
def must_have_name_or_id(self, name, use):
self.mandatory_names_or_ids[name] = use
def final_check(self):
if self.mandatory_fields:
@@ -1943,11 +1945,11 @@ class NameManager:
if not self.validate:
return
for action, use in self.mandatory_names.items():
if action not in self.available_actions and action not in self.available_fields:
for action, use in self.mandatory_names_or_ids.items():
if action not in self.available_actions and action not in self.available_names_or_ids:
msg = _(
"Name '%(name)s' used in '%(use)s' must be present in view but is missing.",
name=action, use=use,
"Name or id '%(name_or_id)s' used in '%(use)s' must be present in view but is missing.",
name_or_id=action, use=use,
)
view.handle_view_error(msg)
odoo/addons/base/tests/test_views.py
+1 -1
View File
@@ -2130,7 +2130,7 @@ class TestViews(ViewCase):
)
self.assertInvalid(
'<form><label for="model"/></form>',
"Name 'model' used in 'label for' must be present in view but is missing.",
"Name or id 'model' used in 'label for' must be present in view but is missing.",
)
def test_col_colspan_numerical(self):