OCA
[MIG] web_widget_x2many_2d_matrix: Migration to 17.0
This does not work at all - it crashes upon loading. Have you tested it at all?
Much better now. Actually we have attempted migration of this as well and came to very similar result. Few remarks on your version:
- The totals only get recomputed on first cell change. It does the same in our version. Don't know why.
- The center alignment of cells in matrix is wrong in my opinion. It's numbers there, they should have been aligned to right.
- The sums are always displayed with 2 decimal figures. This is wrong. In original implementation it used the format of the row/column being summed. So in case of integers, no decimal figures. In case of floats, as many decimal figures as the floats display.
- You have completely removed that boolean field stuff - even from the read-only override check. I was referring to the JS field part causing exceptions but not sure if the read-only oveeride is still needed.
Boolean works. Tested with #2782.
Do all others I mentioned work for you? Because they still do not for me... Please see my comments above.
Here is my testing:
- Totals get recomputed correctly.
- Cell alignment is to the right.
- Float sums: 2 decimals. Integer sums: 0 decimals.
- Boolean field stuff: not sure what to test.
@Rad0van Feel free to clone the PR and make a new commit with your desired changes. Then I guess @SodexisTeam can add it to this PR.
Which of the two PRs should we use? 🤔 This or https://github.com/OCA/web/pull/2744
@SodexisTeam @norlinhenrik
Here is my testing:
- Totals get recomputed correctly.
- Cell alignment is to the right.
- Float sums: 2 decimals. Integer sums: 0 decimals.
- Boolean field stuff: not sure what to test.
@Rad0van Feel free to clone the PR and make a new commit with your desired changes. Then I guess @SodexisTeam can add it to this PR.
Totals get recomputed - nice! Alignment is almost OK - the totals are getting away slightly - see attached picture. Haven't tested integers and booleans yet.
Which of the two PRs should we use? 🤔 This or #2744
@SodexisTeam @norlinhenrik
We use this PR. I just closed #2744.
This PR has the approved label and has been created more than 5 days ago. It should therefore be ready to merge by a maintainer (or a PSC member if the concerned addon has no declared maintainer). 🤖
What a great day to merge this nice PR. Let's do it! Prepared branch 17.0-ocabot-merge-pr-2756-by-pedrobaeza-bump-nobump, awaiting test results.
This PR has the approved label and has been created more than 5 days ago. It should therefore be ready to merge by a maintainer (or a PSC member if the concerned addon has no declared maintainer). 🤖
Congratulations, your PR was merged at 63e85a1051f17afb041fd12dc6ca649929849b46. Thanks a lot for contributing to OCA. ❤️