Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Various fixes, mainly related to list column widths + editable readonly feature #37113

Closed
wants to merge 13 commits into from

Conversation

aab-odoo
Copy link
Contributor

ClassNames 'oe_read_only' (resp. 'oe_edit_only') can be used in
x2many lists to hide columns in 'edit' (resp. 'readonly') modes.

However, before this rev., there were two issues:

  1. Having such a className on a tag didn't work well as
    the 'display: none' rule didn't apply on the body cell (it only
    applied on the button). Same could be observed on
    nodes.
  2. Having more than one invisible column (with those classNames)
    didn't work either, because the footer cells weren't hidden.
    We didn't notice it with one invisible column, because the
    footer contained one cell less than the header and the body.

Those two issues could be observed on the mrp.bom form view.

Task 2068261

Description of the issue/feature this PR addresses:

Current behavior before PR:

Desired behavior after PR is merged:

--
I confirm I have signed the CLA and read the PR guidelines at www.odoo.com/submit-pr

@C3POdoo C3POdoo added the RD research & development, internal work label Sep 19, 2019
@robodoo robodoo added CI 🤖 Robodoo has seen passing statuses and removed CI 🤖 Robodoo has seen passing statuses labels Sep 19, 2019
@aab-odoo
Copy link
Contributor Author

robodoo rebase-ff

@aab-odoo aab-odoo changed the title [FIX] web: correctly handle 'oe_read|edit_only' in lists Various fixes, mainly related to list column widths Sep 19, 2019
@robodoo
Copy link
Contributor

robodoo commented Sep 19, 2019

Merge method set to rebase and fast-forward

@aab-odoo aab-odoo force-pushed the saas-12.5-list-fixes-aab branch 2 times, most recently from 8305017 to 0916f87 Compare September 19, 2019 13:03
@robodoo robodoo added CI 🤖 Robodoo has seen passing statuses and removed CI 🤖 Robodoo has seen passing statuses labels Sep 19, 2019
@robodoo robodoo added the CI 🤖 Robodoo has seen passing statuses label Sep 20, 2019
Copy link
Contributor Author

@aab-odoo aab-odoo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The first two fixes deserve tests :-)

* is overflowing.
*
* @param {number} totalWidth
* @param {number} allowedWidth
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@private

const thElements = [...thead.getElementsByTagName('th')];
const thWidths = thElements.map((th) => th.offsetWidth);
const allowedWidth = table.parentNode.offsetWidth;
const totalWidth = thElements.reduce((total, th) => total + th.offsetWidth, 0);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i'd do this in _trimLargestColumns

@@ -1264,6 +1259,43 @@ ListRenderer.include({
return $(th).outerWidth();
});
},
/**
* Set a maximum width on the largest columns in the list in case the table
* is overflowing.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd explain the whole idea of the algorithm here

this._trimLargestColumns(totalWidth, allowedWidth);
}

const thWidths = thElements.map(th => Number(th.style.maxWidth.split('px')[0]) || th.offsetWidth);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not a fan of the maxWidth.split stuff, maybe we should try to find another way (like storing the max widths we set in another structure)

@robodoo robodoo added CI 🤖 Robodoo has seen passing statuses and removed CI 🤖 Robodoo has seen passing statuses labels Sep 23, 2019
@robodoo robodoo added CI 🤖 Robodoo has seen passing statuses and removed CI 🤖 Robodoo has seen passing statuses labels Sep 23, 2019
@robodoo robodoo removed the CI 🤖 Robodoo has seen passing statuses label Sep 24, 2019
@aab-odoo aab-odoo force-pushed the saas-12.5-list-fixes-aab branch 2 times, most recently from 1269180 to c3cc028 Compare September 24, 2019 08:26
@aab-odoo aab-odoo changed the title Various fixes, mainly related to list column widths Various fixes, mainly related to list column widths + editable readonly feature Sep 24, 2019
@aab-odoo aab-odoo force-pushed the saas-12.5-list-fixes-aab branch 3 times, most recently from c75b767 to 8b93004 Compare September 24, 2019 13:01
Arcasias and others added 5 commits September 25, 2019 09:20
Since the commit allowing all list views to be editable, there are some
views in which we need to ensure that certain fields are always readonly.

Affected modules:
- account
- crm
- product
- purchase
- sale

Task 1967602
Changed the information dipslayed by the confirmation modal in multi edition
in list views.

Before this commit, the modal simply showed a one-line message with the amount of valid records.

Now, there are 3 elements:
- If all of the fields cannot be changed, a first line warns the amount of valid records.
- A second (or first if all fields can be updated) line ensuring that the user is aware of
the occuring changes.
- A table displaying the affected field and the updated value (readonly widget with the new value).

Part of task 1967602
Setting 'overflow: hidden' on the (hidden) textarea used to compute
the (visible) textarea height ensures that there is no vertical
scrollbar, which would lead to a not so precise computation of the
height. The bug could be observed in sale.order form view, in the
order line x2many (description field), and had been introduced by
ef46378.
*:board,hr_skills

Hold on tight, this is a tricky one.

Allows *any* readonly list view (only plain lists, not x2manies) to
become partially editable when checking records.

This means that once checked, a record line acts just like it was
in an editable list. You can click on it, edit any of its values as long as
they're not constrained by readonly modifiers, and save them on the fly.

Clicking on an unchecked record will have the same effect as before:
it will open the record, regardless of the other selected records.

You can also check multiple records and edit them all at once by
changing the value of one selected record (just like standard multi edition).

Keyboard navigation is also allowed between selected records (TAB and SHIFT+TAB
to navigate and ENTER to save).

/!\ If a list is only made of records having all of their fields locked by
readonly modifiers, checking a record and clicking on it will do absolutely
nothing. This is to keep consistency with the rest of the specs.

Task 1967602
Go to Accounting > Accounting > General ledger. The view is grouped
by account. There is a layout issue as the first column (date) is
positionned after the group names, leaving a large blank space.
This is because the date field is considered as an aggregated field
(for sorting purpose, in python), but the JS can't (and we don't
want to) display it. This rev. ensures that such field types are
completely ignored by the JS, s.t. they do not impact the layout.
@robodoo robodoo added the CI 🤖 Robodoo has seen passing statuses label Sep 25, 2019
@VincentSchippefilt
Copy link
Contributor

robodoo r+

@robodoo
Copy link
Contributor

robodoo commented Sep 25, 2019

Linked pull request(s) odoo/enterprise#5745 not ready. Linked PRs are not staged until all of them are ready.

robodoo pushed a commit that referenced this pull request Sep 25, 2019
Go to Accounting > Accounting > General ledger. The view is grouped
by account. There is a layout issue as the first column (date) is
positionned after the group names, leaving a large blank space.
This is because the date field is considered as an aggregated field
(for sorting purpose, in python), but the JS can't (and we don't
want to) display it. This rev. ensures that such field types are
completely ignored by the JS, s.t. they do not impact the layout.

closes #37113

Signed-off-by: VincentSchippefilt <VincentSchippefilt@users.noreply.github.com>
@xmo-odoo
Copy link
Collaborator

@robodoo p=1

@robodoo
Copy link
Contributor

robodoo commented Sep 25, 2019

Merged at 1787ff0, thanks!

@robodoo robodoo closed this Sep 25, 2019
@robodoo robodoo added error 🙅 and removed CI 🤖 Robodoo has seen passing statuses merged 🎉 labels Sep 25, 2019
@robodoo
Copy link
Contributor

robodoo commented Sep 25, 2019

Unable to stage PR ({"message":"Merge conflict","documentation_url":"https://developer.github.com/v3/repos/merging/#perform-a-merge"})

@robodoo robodoo added CI 🤖 Robodoo has seen passing statuses merged 🎉 and removed error 🙅 labels Sep 25, 2019
@Arcasias
Copy link
Contributor

Arcasias commented Oct 9, 2019

@jpp-odoo commit for textarea line return: 67cccb5

@jpp-odoo
Copy link
Contributor

jpp-odoo commented Oct 9, 2019

@Arcasias Thanks!

@fw-bot fw-bot deleted the saas-12.5-list-fixes-aab branch October 18, 2019 10:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI 🤖 Robodoo has seen passing statuses RD research & development, internal work
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants