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.

Sign up for GitHub

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

Jump to bottom

[16.0][IMP] web_company_color: Forward port #2209 from 14.0 #2534

Merged
merged 2 commits into from
Nov 6, 2023
Merged

[16.0][IMP] web_company_color: Forward port #2209 from 14.0 #2534

merged 2 commits into from
Nov 6, 2023

Conversation

aleuffre
Copy link
Contributor

Cherry pick of d59008f and 8c8ffe7

@pedrobaeza pedrobaeza added this to the 16.0 milestone Jun 20, 2023
@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). 🤖

@aleuffre
Copy link
Contributor Author

aleuffre commented Jul 5, 2023 •
edited
Loading

For completeness' sake, I feel like I should mention that the module doesn't work as cleanly as it used to in v14 and below. See also the forward port of the same commits v15, that suffers from similar issues.

#2467

@dreispt
Copy link
Member

dreispt commented Nov 6, 2023

/ocabot merge minor

@OCA-git-bot
Copy link
Contributor

Hey, thanks for contributing! Proceeding to merge this for you.
Prepared branch 16.0-ocabot-merge-pr-2534-by-dreispt-bump-minor, awaiting test results.

OCA-git-bot added a commit that referenced this pull request Nov 6, 2023
@OCA-git-bot
Merge PR #2534 into 16.0
4bcd770 
Signed-off-by dreispt
@OCA-git-bot
Copy link
Contributor

It looks like something changed on 16.0 in the meantime.
Let me try again (no action is required from you).
Prepared branch 16.0-ocabot-merge-pr-2534-by-dreispt-bump-minor, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit 238d43c into OCA:16.0 Nov 6, 2023
@OCA-git-bot
Copy link
Contributor

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

@LauraCForgeFlow LauraCForgeFlow mentioned this pull request Jan 16, 2024
Open
@LoisRForgeFlow
Copy link
Contributor

Hi all,

it would be great if you could drop your opinion on this issue #2721.

Thanks in advance.

hbrunn
hbrunn reviewed Feb 12, 2024
View reviewed changes
web_company_color/models/res_company.py
"color_link_text": values.get("color_link_text")
or 'theme-color("primary")',
"color_link_text_hover": values.get("color_link_text_hover")
or 'darken(theme-color("primary"), 10%)',
Copy link
Member

Choose a reason for hiding this comment

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

you might want to double check if those fallbacks are still the default values in this version, otherwise there'll be weird effects when not all values are set by the user

@aleuffre aleuffre deleted the 16.0-company-colors-imp branch February 12, 2024 11:46
@anajuaristi
Copy link

anajuaristi commented Mar 1, 2024 •
edited
Loading

Hello
Testing this great module in V16, I can say that today, it's needed to fill any color field on company form to make it work but setting them properly it works really fine.
Thank you all!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hbrunn hbrunn hbrunn left review comments

@CasVissers-360ERP CasVissers-360ERP CasVissers-360ERP approved these changes

@pedrobaeza pedrobaeza pedrobaeza approved these changes

Assignees
No one assigned
Labels
approved merged 🎉 ready to merge
Projects
None yet
Milestone
16.0
Development

Successfully merging this pull request may close these issues.
8 participants
@aleuffre @OCA-git-bot @dreispt @LoisRForgeFlow @anajuaristi @hbrunn @pedrobaeza @CasVissers-360ERP