From bf3c1d3e0c2cb9f6ed560994d24de026c080aa01 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=A9ry=20Debongnie?= Date: Fri, 9 Aug 2019 08:07:55 +0000 Subject: [PATCH] [IMP] web: editable list: column's widths This rev. refines the way we compute column's widths in editable list views. When there are displayed records, we let the browser compute the width of each column (which is thus optimal with respect to the content), and we then freeze those widths s.t. it doesn't flicker when we switch a row in edition. When there is no record, we keep the former heuristic based on absolute and relative widths (depending on field types). However, we set the same weight for all fields having a relative width (i.e. all but boolean, date(time) and numeric), such that the remaining space is evenly distributed between them. Co-authored-by: Aaron Bohy --- .../static/src/js/fields/relational_fields.js | 30 ++++++- .../js/views/list/list_editable_renderer.js | 85 +++++++++++++++---- .../static/src/js/views/list/list_renderer.js | 1 - addons/web/static/src/scss/list_view.scss | 6 +- addons/web/static/tests/views/list_tests.js | 39 ++++++++- 5 files changed, 139 insertions(+), 22 deletions(-) diff --git a/addons/web/static/src/js/fields/relational_fields.js b/addons/web/static/src/js/fields/relational_fields.js index fcae2043d00..eefb771c69a 100644 --- a/addons/web/static/src/js/fields/relational_fields.js +++ b/addons/web/static/src/js/fields/relational_fields.js @@ -17,10 +17,11 @@ var AbstractField = require('web.AbstractField'); var basicFields = require('web.basic_fields'); var concurrency = require('web.concurrency'); var ControlPanelView = require('web.ControlPanelView'); -var dialogs = require('web.view_dialogs'); var core = require('web.core'); var data = require('web.data'); var Dialog = require('web.Dialog'); +var dialogs = require('web.view_dialogs'); +var dom = require('web.dom'); var KanbanRecord = require('web.KanbanRecord'); var KanbanRenderer = require('web.KanbanRenderer'); var ListRenderer = require('web.ListRenderer'); @@ -905,6 +906,22 @@ var FieldX2Many = AbstractField.extend({ start: function () { return this._renderControlPanel().then(this._super.bind(this)); }, + /** + * For the list renderer to properly work, it must know if it is in the DOM, + * and be notified when it is attached to the DOM. + */ + on_attach_callback: function () { + this.isInDOM = true; + if (this.renderer) { + this.renderer.on_attach_callback(); + } + }, + /** + * For the list renderer to properly work, it must know if it is in the DOM. + */ + on_detach_callback: function () { + this.isInDOM = false; + }, //-------------------------------------------------------------------------- // Public @@ -1096,7 +1113,16 @@ var FieldX2Many = AbstractField.extend({ this.renderer = new Renderer(this, this.value, rendererParams); this.$el.addClass('o_field_x2many o_field_x2many_' + viewType); - return this.renderer ? this.renderer.appendTo(this.$el) : this._super(); + if (this.renderer) { + return this.renderer.appendTo(document.createDocumentFragment()).then(function () { + dom.append(self.$el, self.renderer.$el, { + in_DOM: self.isInDOM, + callbacks: [{widget: self.renderer}], + }); + }); + } else { + return this._super(); + } }, /** * Instanciates a control panel with the appropriate buttons and a pager. 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 65f5b433c27..9275d414578 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 @@ -43,6 +43,8 @@ ListRenderer.include({ var self = this; this._super.apply(this, arguments); + this.editable = params.editable; + // if addCreateLine (resp. addCreateLineInGroups) is true, the renderer // will add a 'Add a line' link at the bottom of the list view (resp. // at the bottom of each group) @@ -107,6 +109,29 @@ ListRenderer.include({ } return this._super(); }, + /** + * The list renderer needs to know if it is in the DOM, and to be notified + * when it is attached to the DOM to properly compute column widths. + * + * @override + */ + on_attach_callback: function () { + this.isInDOM = true; + if (this.editable) { + this._freezeColumnWidths(); + } + this._super(); + }, + /** + * The list renderer needs to know if it is in the DOM to properly compute + * column widths. + * + * @override + */ + on_detach_callback: function () { + this.isInDOM = false; + this._super(); + }, //-------------------------------------------------------------------------- // Public @@ -493,9 +518,39 @@ ListRenderer.include({ } }, /** - * Returns the relative width according to the widget or the field type. - * @see _renderHeader + * This function freezes the column widths and forces a fixed table-layout, + * once the browser has computed the optimal width of each column according + * to the displayed records. We want to freeze widths s.t. it doesn't + * flicker when we switch a row in edition. * + * We skip this when there is no record as we don't want to fix widths + * according to column's labels. In this case, we fallback on the 'weight' + * heuristic, which assigns to each column a fixed or relative width + * depending on the widget or field type. + * + * Note that the list must be in the DOM when this function is called. + * + * @private + */ + _freezeColumnWidths: function () { + if (!this.state.data.length) { + // there is no record -> don't force column's widths w.r.t. their label + return; + } + var $thead = this.$('thead'); + $thead.find('th').each(function () { + var $th = $(this); + $th.css('width', $th.outerWidth() + 'px'); + }); + this.$('table').css('table-layout', 'fixed'); + }, + /** + * Returns the relative width according to the widget or the field type. + * This is only used when there is no record in the list (i.e. when we can't + * let the browser compute the optimal width of each column). + * + * @see _renderHeader + * @private * @param {Object} column an arch node * @returns {integer | string} either a weight factor (number) or a css * width description (string) @@ -516,22 +571,13 @@ ListRenderer.include({ return widget.widthFactor; } switch (field.type) { - case 'binary': return 1; case 'boolean': return '40px'; - case 'char': return 2; case 'date': return '92px'; case 'datetime': return '146px'; case 'float': return '92px'; - case 'html': return 3; case 'integer': return '74px'; - case 'many2many': return 2; - case 'many2one': return 2; case 'monetary': return '104px'; - case 'one2many': return 2; - case 'reference': return 1.5; - case 'selection': return 1.5; - case 'text': return 2.5; - default: return 1; + default: return 1; // evenly distribute the remaining space } }, /** @@ -842,7 +888,7 @@ ListRenderer.include({ _renderHeader: function () { var $thead = this._super.apply(this, arguments); - if (this.editable) { + if (this.editable && !this.state.data.length) { // we compute the sum of the weights for each columns, excluding // those with a fixed width. var totalWidth = this.columns.reduce(function (acc, column) { @@ -934,8 +980,17 @@ ListRenderer.include({ this.currentRow = null; this.allRecordsIds = null; return this._super.apply(this, arguments).then(function () { - if (self.editable) { - self.$('table').addClass('o_editable_list'); + var table = self.el.getElementsByTagName('table')[0]; + if (table) { // no table if no content helper displayed + if (!self.state.data.length) { // FIXME: only handles ungrouped case + table.classList.add('o_empty_list'); + } + if (self.editable) { + table.classList.add('o_editable_list'); + if (self.isInDOM) { + self._freezeColumnWidths(); + } + } } }); }, 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 fda8d12f9e4..98c8bf71046 100644 --- a/addons/web/static/src/js/views/list/list_renderer.js +++ b/addons/web/static/src/js/views/list/list_renderer.js @@ -67,7 +67,6 @@ var ListRenderer = BasicRenderer.extend({ this.hasSelectors = params.hasSelectors; this.selection = params.selectedRecords || []; this.pagers = []; // instantiated pagers (only for grouped lists) - this.editable = params.editable; this.isGrouped = this.state.groupedBy.length > 0; this.groupbys = params.groupbys; }, diff --git a/addons/web/static/src/scss/list_view.scss b/addons/web/static/src/scss/list_view.scss index f2fc83902a4..148ec6af348 100644 --- a/addons/web/static/src/scss/list_view.scss +++ b/addons/web/static/src/scss/list_view.scss @@ -55,7 +55,7 @@ } } - .o_column_sortable { + .o_column_sortable:not(.o_handle_cell) { position: relative; user-select: none; // Prevent unwanted selection while sorting @@ -180,7 +180,9 @@ //-------------------------------------------------------------------------- &.o_editable_list { - table-layout: fixed; + &.o_empty_list { + table-layout: fixed; + } .o_list_record_remove_header, .o_handle_cell { width: 32px; diff --git a/addons/web/static/tests/views/list_tests.js b/addons/web/static/tests/views/list_tests.js index f4eb264241d..8397364dd2f 100644 --- a/addons/web/static/tests/views/list_tests.js +++ b/addons/web/static/tests/views/list_tests.js @@ -230,9 +230,10 @@ QUnit.module('Views', { list.destroy(); }); - QUnit.test('editable rendering with handle', async function (assert) { + QUnit.test('editable rendering with handle and no data', async function (assert) { assert.expect(6); + this.data.foo.records = []; var list = await createView({ View: ListView, model: 'foo', @@ -1429,7 +1430,40 @@ QUnit.module('Views', { list.destroy(); }); - QUnit.test('width of some of the fields should be hardcoded', async function (assert) { + QUnit.test('column widths should depend on the content when there is data', async function (assert) { + assert.expect(1); + + this.data.foo.records[0].foo = 'Some very very long value for a char field'; + + var list = await createView({ + View: ListView, + model: 'foo', + data: this.data, + arch: '' + + '' + + '' + + '' + + '' + + '' + + '' + + '', + viewOptions: { + limit: 2, + }, + }); + + var widthPage1 = list.$(`th[data-name=foo]`)[0].offsetWidth; + + await testUtils.dom.click(list.$('.o_pager_next')); + + var widthPage2 = list.$(`th[data-name=foo]`)[0].offsetWidth; + assert.ok(widthPage1 > widthPage2, + 'column widths should be computed dynamically according to the content'); + + list.destroy(); + }); + + QUnit.test('width of some of the fields should be hardcoded if no data', async function (assert) { const assertions = [ { field: 'bar', expected: 40, type: 'Boolean' }, { field: 'int_field', expected: 74, type: 'Integer' }, @@ -1439,6 +1473,7 @@ QUnit.module('Views', { ]; assert.expect(assertions.length + 1); + this.data.foo.records = []; var list = await createView({ View: ListView, model: 'foo',