From 58790d58e1e313be46584892f80c27244bf3bf39 Mon Sep 17 00:00:00 2001 From: AH-Yussef Date: Wed, 6 Mar 2024 03:39:09 +0200 Subject: [PATCH] [FIX] web: correctly align column values with column header Steps to reproduce: - Install planning app. - Go to planning app, and convert the view to List. - Group by Resource -for example-. - Notice how the allocated time values are mis-aligned with the header label. Investigation: - in the planning tree view, notice the property `open_form_view` set to `True` https://github.com/odoo/enterprise/blob/418ea2114c7d935865dc52e5fd5a30d626987190/planning/views/planning_views.xml#L8 - In the `getGroupNameCellColSpan` method: - the `firstAggregateIndex` is `1` belonging to the `allocated_hours` (Allocated Time)column which set the `colspan = firstAggregateIndex = 1` - as `this.hasSelectors` is `true`, `colspan = colspan+1 = 2` (which is the correct value) - but now as the `open_form_view` was set to `True`, the `this.props.onOpenFormView` exists and so the `colspan = colspan+1 = 3` which makes the value be misaligned to the right by one column. The proposed fix: - As the `open_form_view` option add a View button to the end of the row, it makes sense to do the check with the `lastAggregateIndex` inside `getGroupPagerCellColspan` method. - In our case: - the `lastAggregateIndex` is `1` belonging also to the `allocated_hours` which set the `colspan = this.state.columns.length - lastAggregateIndex - 1 = 3 - 1 - 1 = 1` - as we don't display optional fields this check fails https://github.com/odoo/odoo/blob/5af3cfc2b9ce648ae4aff69150266e4ad519050d/addons/web/static/src/views/list/list_renderer.js#L1025-L1027 and the `colspan = 1` - Now we check the `this.props.onOpenFormView` condition which set `colspan = colspan+1 = 2` which is correct. - But what if we wanted to show the optional field `allocated_percentage` would this fix still work? Yes! - the `lastAggregateIndex` is `2` belonging to the `allocated_percentage` which set the `colspan = this.state.columns.length - lastAggregateIndex - 1 = 3 - 2 - 1 = 0` - as we do display an optional fields= this check works https://github.com/odoo/odoo/blob/5af3cfc2b9ce648ae4aff69150266e4ad519050d/addons/web/static/src/views/list/list_renderer.js#L1025-L1027 and the `colspan = colspan+1 = 2` - Now we check the `this.props.onOpenFormView` condition which set `colspan = colspan+1 = 3` which is correct. - One last case, what if we have no aggregates - both firstAggregateIndex` = `lastAggregateIndex` = -1 - we go into the else clause inside `getGroupPagerCellColspan`, making the `colspan = this.state.columns.length > 1 ? DEFAULT_GROUP_PAGER_COLSPAN : 0 = True ? DEFAULT_GROUP_PAGER_COLSPAN : 0 = DEFAULT_GROUP_PAGER_COLSPAN = 1 - if the `open_form_view` was `True`, the colspan becomes `colspan = colspan+1 = 2`. opw-3764616 closes odoo/odoo#156647 Signed-off-by: Aaron Bohy (aab) --- .../static/src/views/list/list_renderer.js | 13 ++--- .../web/static/tests/views/list_view_tests.js | 54 +++++++++++++++++++ 2 files changed, 61 insertions(+), 6 deletions(-) diff --git a/addons/web/static/src/views/list/list_renderer.js b/addons/web/static/src/views/list/list_renderer.js index 8fbf3544d49..12d8b047259 100644 --- a/addons/web/static/src/views/list/list_renderer.js +++ b/addons/web/static/src/views/list/list_renderer.js @@ -1012,23 +1012,24 @@ export class ListRenderer extends Component { if (this.hasSelectors) { colspan++; } - if (this.props.onOpenFormView) { - colspan++; - } return colspan; } getGroupPagerCellColspan(group) { const lastAggregateIndex = this.getLastAggregateIndex(group); + let colspan; if (lastAggregateIndex > -1) { - let colspan = this.state.columns.length - lastAggregateIndex - 1; + colspan = this.state.columns.length - lastAggregateIndex - 1; if (this.displayOptionalFields) { colspan++; } - return colspan; } else { - return this.state.columns.length > 1 ? DEFAULT_GROUP_PAGER_COLSPAN : 0; + colspan = this.state.columns.length > 1 ? DEFAULT_GROUP_PAGER_COLSPAN : 0; } + if (this.props.onOpenFormView) { + colspan++; + } + return colspan } getGroupPagerProps(group) { diff --git a/addons/web/static/tests/views/list_view_tests.js b/addons/web/static/tests/views/list_view_tests.js index 8ac94b40b69..cdc111b166a 100644 --- a/addons/web/static/tests/views/list_view_tests.js +++ b/addons/web/static/tests/views/list_view_tests.js @@ -2141,6 +2141,60 @@ QUnit.module("Views", (hooks) => { } ); + QUnit.test( + "basic grouped list rendering 4 cols with aggregates, selector and openFormView", + async function (assert) { + await makeView({ + type: "list", + resModel: "foo", + serverData, + arch: ` + + + + + + `, + groupBy: ["bar"], + }); + + assert.strictEqual( + target.querySelector(".o_group_header th").getAttribute("colspan"), "2" + ); + assert.strictEqual( + target.querySelector(".o_group_header th:last-child").getAttribute("colspan"), + "2", + ); + } + ); + + QUnit.test( + "basic grouped list rendering 4 cols with aggregates, selector, optional and openFormView", + async function (assert) { + await makeView({ + type: "list", + resModel: "foo", + serverData, + arch: ` + + + + + + `, + groupBy: ["bar"], + }); + + assert.strictEqual( + target.querySelector(".o_group_header th").getAttribute("colspan"), "2" + ); + assert.strictEqual( + target.querySelector(".o_group_header th:last-child").getAttribute("colspan"), + "1", + ); + } + ); + QUnit.test("group a list view with the aggregable field 'value'", async function (assert) { serverData.models.foo.fields.value = { string: "Value",