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

[17.0][MIG] web_tree_dynamic_colored_field #2771

Merged
merged 32 commits into from
Apr 3, 2024

Conversation

dz0
Copy link

@dz0 dz0 commented Mar 15, 2024

No description provided.

damdam-s and others added 30 commits March 12, 2024 13:47
support color_field attribute

update manifest

higher version number bump

typo
The options in <field> attributes are parsed as python expressions:
https://github.com/odoo/odoo/blob/d18976d7489f6cd1c904f72557104807233a178d/addons/web/static/src/js/services/data_manager.js#L273

And the options in <button> are parsed as json...
https://github.com/odoo/odoo/blob/d18976d7489f6cd1c904f72557104807233a178d/addons/web/static/src/js/services/data_manager.js#L411

This code only support the <field> element because I'm not sure there
is a use for the <button> element.
…'color_field'

The index at 0 in the following code:
  var colorField = this.arch.attrs.colors.split(';')
  .filter(color => color.trim().startsWith('color_field'))[0]
Was failing on such valid xml:
  <tree string="Buffer monitor"
        colors="red:procure_recommended_qty &gt; 0">
Use 'options' instead of 'colors' on tree views

The colors attribute has been removed from the RelaxNG schema in
Odoo [0], so use the 'options' instead.

Closes OCA#1479

[0] odoo/odoo@7024f8d#diff-e9acd2f731cc01f302055b6e232df983
The 'colors' and 'options' fields are not supported by the RelaxNG
schema of the <tree> element. Remove the support until we find a
good solution for this.
[UPD] README.rst
Update translation files

Updated by "Update PO files to match POT (msgmerge)" hook in Weblate.

Translation: web-13.0/web-13.0-web_tree_dynamic_colored_field
Translate-URL: https://translation.odoo-community.org/projects/web-13-0/web-13-0-web_tree_dynamic_colored_field/
[UPD] README.rst
Update translation files
Updated by "Update PO files to match POT (msgmerge)" hook in Weblate.

Translation: web-15.0/web-15.0-web_tree_dynamic_colored_field
Translate-URL: https://translation.odoo-community.org/projects/web-15-0/web-15-0-web_tree_dynamic_colored_field/

Added translation using Weblate (French)
Copy link

@AndreuOForgeFlow AndreuOForgeFlow left a comment

Choose a reason for hiding this comment

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

I have checked locally but I can't seem to make it work. I did, as an example, something like this using another view from an OCA module:
<field name="code" options="{'bg_color': 'red:delivery_type != \'fixed\''}"/>

I used it the same way on the previous version (v15) and it works.

Do you have an example where it works? Am I doing something wrong with the notation?

Thanks in advance!

@dz0 dz0 force-pushed the 17.0-mig-web_tree_dynamic_colored_field branch from f907634 to 7b94edd Compare March 27, 2024 11:09
@dz0
Copy link
Author

dz0 commented Mar 27, 2024

My bad, didn't test for the last time -- was a typo/gotcha (now fixed).

Copy link

@AndreuOForgeFlow AndreuOForgeFlow left a comment

Choose a reason for hiding this comment

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

Just a small comment. Other than that LGTM! Code and functional review

Copy link

@GuillemCForgeFlow GuillemCForgeFlow left a comment

Choose a reason for hiding this comment

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

Functional review LGTM 👍🏿
Just one small thing, could you update the PR title to match the module name?

@OCA-git-bot
Copy link
Contributor

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). 🤖

@AndreuOForgeFlow
Copy link

@dz0, also please squash the last 2 commits. A migration can only have the pre-commit commit and the migration commit. And change the title of the PR to the name of the module: web_tree_dynamic_colored_field

@dz0 dz0 changed the title [17.0][MIG] tree_dynamic_colored_field [17.0][MIG] web_tree_dynamic_colored_field Apr 2, 2024
- Implementation now is based on updating `style` attribute of `cell/td` instead of  manipulating element's `css` value in js.

- Cleanup docs about no more functioning  `colors` parameter for `tree` (since 13.0)
@dz0 dz0 force-pushed the 17.0-mig-web_tree_dynamic_colored_field branch from 6a631d7 to 906bb81 Compare April 2, 2024 10:13
@dz0
Copy link
Author

dz0 commented Apr 2, 2024

please squash the last 2 commits.

done

And change the title of the PR to the name of the module: web_tree_dynamic_colored_field

done :)

@AndreuOForgeFlow
Copy link

please squash the last 2 commits.

done

And change the title of the PR to the name of the module: web_tree_dynamic_colored_field

done :)

Perfect, thank you for the changes!

I guess this is ready to merge, @etobella?

Copy link
Member

@etobella etobella left a comment

Choose a reason for hiding this comment

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

Tested and worked properly

/ocabot migration web_tree_dynamic_colored_field

@OCA-git-bot OCA-git-bot added this to the 17.0 milestone Apr 3, 2024
@OCA-git-bot OCA-git-bot mentioned this pull request Apr 3, 2024
26 tasks
@etobella
Copy link
Member

etobella commented Apr 3, 2024

/ocabot merge nobump

@OCA-git-bot
Copy link
Contributor

This PR looks fantastic, let's merge it!
Prepared branch 17.0-ocabot-merge-pr-2771-by-etobella-bump-nobump, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit 19a9e08 into OCA:17.0 Apr 3, 2024
7 checks passed
@OCA-git-bot
Copy link
Contributor

Congratulations, your PR was merged at bc43787. Thanks a lot for contributing to OCA. ❤️

@zamberjo
Copy link
Member

zamberjo commented May 7, 2024

Hi @dz0! can you backport to v16.0¿?

@dz0
Copy link
Author

dz0 commented May 7, 2024

Hi @dz0! can you backport to v16.0¿?

Not sure if will find time soon.
My company intends to use it only in v17.

Maybe you can try same code in v16 (just change version in manifest)?

@cvinh
Copy link
Contributor

cvinh commented Jul 5, 2024

@zamberjo can you please review the backport PR from 17.0 to 16.0 #2873

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.