Skip to content

[16.0][MIG] web_widget_char_size: Migration to 16.0#3008

Merged
OCA-git-bot merged 7 commits intoOCA:16.0from
PyTech-SRL:16.0-mig-web_widget_char_size
Dec 16, 2024
Merged

[16.0][MIG] web_widget_char_size: Migration to 16.0#3008
OCA-git-bot merged 7 commits intoOCA:16.0from
PyTech-SRL:16.0-mig-web_widget_char_size

Conversation

@anusriNPS
Copy link
Contributor

Migrating web_widget_char_size to 16.0

@anusriNPS anusriNPS force-pushed the 16.0-mig-web_widget_char_size branch from 7a1e6c1 to 2757a21 Compare December 2, 2024 09:32
@anusriNPS anusriNPS force-pushed the 16.0-mig-web_widget_char_size branch 2 times, most recently from 16f884f to 704616d Compare December 9, 2024 15:24
@anusriNPS anusriNPS force-pushed the 16.0-mig-web_widget_char_size branch from 704616d to f48ad0b Compare December 11, 2024 16:37
Copy link
Contributor

@HekkiMelody HekkiMelody 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

Copy link

@renda-dev renda-dev left a comment

Choose a reason for hiding this comment

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

Code review, LGTM

const _extractProps = CharField.extractProps;
CharField.extractProps = ({attrs, field}) => {
return Object.assign(_extractProps({attrs, field}), {
maxLength: field.size || attrs.options.size,

Choose a reason for hiding this comment

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

nitpick(non-blocking): According to https://developer.mozilla.org/en-US/docs/Web/HTML/Attributes/maxlength it should be without the capital 'L'

Still works anyways tho

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Required to update actual definition in order to be aligned with HTML attributes name:
https://github.com/odoo/odoo/blob/16.0/addons/web/static/src/views/fields/char/char_field.js#L80

Choose a reason for hiding this comment

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

Everything is clear now, it's good as it is, thanks!

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

@renda-dev
Copy link

Greetings @OCA/web-maintainers, would anyone mind giving this one a look?

I personally think this is pretty useful :)

Copy link
Contributor

@CarlosRoca13 CarlosRoca13 left a comment

Choose a reason for hiding this comment

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

Thank you for your effort! Please check that when fields have a widget assigned, it is not working. For example, by adding the options to the phone field in the res.partner form. Try loading the asset right after the char_field is loaded. Here’s how to do it: https://www.odoo.com/documentation/17.0/es/developer/reference/frontend/assets.html#after

You also have examples in the code 😄

@HekkiMelody
Copy link
Contributor

Thank you for your effort! Please check that when fields have a widget assigned, it is not working. For example, by adding the options to the phone field in the res.partner form. Try loading the asset right after the char_field is loaded. Here’s how to do it: https://www.odoo.com/documentation/17.0/es/developer/reference/frontend/assets.html#after

You also have examples in the code 😄

I didn't even know this was a thing, I thought it was a given that it would only work with the default char widget.

Thank you for pointing it out!

@anusriNPS
Copy link
Contributor Author

anusriNPS commented Dec 16, 2024

Thank you for your effort! Please check that when fields have a widget assigned, it is not working. For example, by adding the options to the phone field in the res.partner form. Try loading the asset right after the char_field is loaded. Here’s how to do it: https://www.odoo.com/documentation/17.0/es/developer/reference/frontend/assets.html#after

You also have examples in the code 😄

Hi,

I tried to implement as suggested by keeping the web_widget_char_size definition after target file of actual char_field.js file. But, it did not help to use option="{'size': 10}" from the view definition.
I also tried to check option="{'size': 10}" usage in phone field in the contact module in version 14. But, i did not see it's usage in version 14 as well.
form_view_widget_phonev14
phone_uses_FieldPhone

I also think that in order to use "option={size: 10}" in the char field which uses "widget=phone", updating PhoneField, FormPhoneField definition would help to achieve the same.

As version 14 does not support this feature from web_widget_char_size module, is this addition required here?

Please let me know your views on this observation and also correct me if I am wrong.

Copy link
Contributor

@CarlosRoca13 CarlosRoca13 left a comment

Choose a reason for hiding this comment

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

Okay, I see that PhoneField is not extending CharField. It’s a bit inconvenient, but I think we can do without it. If someone needs it in the future, we could create a web_widget_phone_size module.

/ocabot migration web_widget_char_size
/ocabot merge nobump

@OCA-git-bot OCA-git-bot added this to the 16.0 milestone Dec 16, 2024
@OCA-git-bot
Copy link
Contributor

On my way to merge this fine PR!
Prepared branch 16.0-ocabot-merge-pr-3008-by-CarlosRoca13-bump-nobump, awaiting test results.

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

@OCA-git-bot OCA-git-bot mentioned this pull request Dec 16, 2024
47 tasks
@OCA-git-bot OCA-git-bot merged commit 347f0f9 into OCA:16.0 Dec 16, 2024
6 checks passed
@OCA-git-bot
Copy link
Contributor

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

@HekkiMelody HekkiMelody deleted the 16.0-mig-web_widget_char_size branch June 12, 2025 09:30
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.

8 participants