[IMP] web_field_company_dependent_flag: Better icon rendering - #1053
legalsylvain wants to merge 2 commits into
Conversation
Otherwise the icon is shown within the value space of the field and also influences the font used.
OCA-git-bot
added
series:16.0
mod:company_dependent_flag
9ec9123 to
296ccad
Compare
SirPyTech
left a comment
SirPyTech
left a comment
There was a problem hiding this comment.
Please keep the cherry-picked commit as similar as possible to the original commit 8c18c5b (now e6f1f66).
praise: Using fields_get instead of _get_view feels less clunky 👍
issue: Any idea why I can't see the icon in runboat?
http://oca-multi-company-16-0-pr1053-296ccad38c86.runboat.odoo-community.org/web?debug=assets#id=23&cids=1&menu_id=111&action=249&model=product.template&view_type=form
| def _get_view(self, view_id=None, view_type="form", **options): | ||
| arch, view = super()._get_view(view_id, view_type, **options) | ||
| if view_type == "form": | ||
| self._update_company_dependent_css(arch) | ||
| return arch, view | ||
|
|
||
| def _update_company_dependent_css(self, arch): | ||
| cpny_dep_fields = [ | ||
| def fields_get(self, allfields=None, attributes=None): | ||
| result = super().fields_get(allfields=allfields, attributes=attributes) |
| "depends": [ | ||
| "base", | ||
| ], | ||
| "depends": ["web"], |
There was a problem hiding this comment.
question: Is this really needed? If so, should I have it in #1052 too?
There was a problem hiding this comment.
I don't think if this change is necessary but the module does'nt work without web installed : https://github.com/grap/multi-company/blob/16.0-FIX-company_dependent_flag-SLG/company_dependent_flag/static/src/form_label.esm.js#L6-L7
If so, should I have it in #1052 too?
Yes, ideally.
There was a problem hiding this comment.
👍 added
I don't think I have ever seen an installation without web though
| """Inherit to apply your own class""" | ||
|
|
||
| return ["fa", "fa-building-o", "d-flex", "flex-row"] | ||
| return ["fa", "fa-lg", "fa-building-o", "company_dependent_field_icon"] |
There was a problem hiding this comment.
suggestion: fa-lg is increasing the height of the whole row, that does not look good, can we drop it?
There was a problem hiding this comment.
I didn't saw ! thanks for the review. I removed fa-lg.
| """Inherit to apply your own class""" | ||
|
|
||
| return ["fa", "fa-building-o", "d-flex", "flex-row"] | ||
| return ["fa", "fa-lg", "fa-building-o", "company_dependent_field_icon"] |
There was a problem hiding this comment.
praise: The dedicated class will help when we want to further customize the element 👏
is a good reason ! I don't know why, but since some days, I have to manually install modules on OCA runboat... |
- put a margin to avoid to join text and building icon. - add test to avoid to display the buidling icon, if user is not member of multicompany group - add fa-lg as in odoo core
296ccad to
1cadf16
Compare
Hum. sorry, I don't know exactly how to split a commit in two commit, when there are other commit on top of this one. Is a blocking problem, as this branch is not the "head" one, and this "problem" will be forgotten in more recent version ? |
There was a problem hiding this comment.
issue: Any idea why I can't see the icon in runboat?
is a good reason !
I don't know why, but since some days, I have to manually install modules on OCA runboat...
🤦♂️ I should have checked, sorry for the noise, I confirm it works correctly when the module is installed: 
you can use this image for the README so it comes from 16.0 and shows the tooltip 😉
For your info, no module is installed in runboat when there are rebel modules, that is the case for this repository:
multi-company/.copier-answers.yml
Lines 19 to 21 in 8537922
Please keep the cherry-picked commit as similar as possible to the original commit 8c18c5b (now e6f1f66).
Hum. sorry, I don't know exactly how to split a commit in two commit, when there are other commit on top of this one. Is a blocking problem, as this branch is not the "head" one, and this "problem" will be forgotten in more recent version ?
I'm sorry but I'd like the commit I authored to be accurate with what I did so for me it's blocking.
A small change is fine but switching from _get_view to fields_get was not the purpose of that commit.
You can add yourself as a co-author and keep it as-is, or you can cherry-pick it again, and then apply your commits on top of it: in case of conflicts just accept your code.
For your info, the right way to split the commit in two is: git rebase, choose edit for that commit, create the new commits as you like, continue the rebase.
| """Inherit to apply your own class""" | ||
|
|
||
| return ["fa", "fa-building-o", "d-flex", "flex-row"] | ||
| return ["fa", "fa-lg", "fa-building-o", "company_dependent_field_icon"] |
3 participants
Otherwise the icon is shown within the value space of the field and also influences the font used.
backport of 8c18c5b done by @SirPyTech in the migration (and rename of
web_field_company_dependent_flag)company_dependent_flagmodulealso apply some simplification, overloading
fields_getinstead of_get_view.It makes the code more easy.
This implementation is based on the web_field_tooltip implementation. See : https://github.com/OCA/web/blob/16.0/web_field_tooltip/models/base.py#L28
apply same display as in odoo core.
Before
After