Skip to content

[14.0][IMP] web_company_color: Forward port #1903 from 12.0#2209

Merged
OCA-git-bot merged 2 commits into
OCA:14.0from
HekkiMelody:14.0-company-colors-imp
Oct 4, 2022
Merged

[14.0][IMP] web_company_color: Forward port #1903 from 12.0#2209
OCA-git-bot merged 2 commits into
OCA:14.0from
HekkiMelody:14.0-company-colors-imp

Conversation

@HekkiMelody

Copy link
Copy Markdown
Contributor

Forward port of #1903
commit e18209c

Comment thread web_company_color/models/res_company.py Outdated
"color_link_text",
"color_link_text_hover",
)
if "logo" in values:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

You seem to be undoing https://github.com/OCA/web/pull/2158/files. Is that on purpose?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Ah, good catch. I didn't really follow this module's history and just ported the commit trying to preserve as much of it as possible.

Thank you 🙏
Fixing it now.

@HekkiMelody

HekkiMelody commented Jul 6, 2022

Copy link
Copy Markdown
Contributor Author

Added a commit to fix #2248

@StefanRijnhart

Copy link
Copy Markdown
Member

@hbrunn your improvement is being forward ported! Care to review?

@OCA-git-bot

Copy link
Copy Markdown
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). 🤖

1 similar comment
@OCA-git-bot

Copy link
Copy Markdown
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). 🤖

@StefanRijnhart

Copy link
Copy Markdown
Member

/ocabot merge minor

@OCA-git-bot

Copy link
Copy Markdown
Contributor

Hey, thanks for contributing! Proceeding to merge this for you.
Prepared branch 14.0-ocabot-merge-pr-2209-by-StefanRijnhart-bump-minor, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit 9699346 into OCA:14.0 Oct 4, 2022
@OCA-git-bot

Copy link
Copy Markdown
Contributor

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

@LoisRForgeFlow LoisRForgeFlow left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm late to this, but I just hit into this change. I see 2 problems:

  1. Starting with the field in blank you need to set all fields in order for it to have an effect. (you cannot just set the navbar colors for instance).
  2. When computing colors from logo, only the nabvar ones are filled.

@HekkiMelody HekkiMelody deleted the 14.0-company-colors-imp branch November 10, 2022 08:52
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.

5 participants