Skip to content

[18.0][FIX] web_company_color: avoid showing other company's colors and apply all configured colors correctly - #3759

Merged
OCA-git-bot merged 2 commits into
OCA:18.0from
Tecnativa:18.0-FIX-web_company_color
Oct 28, 2026
Merged

OCA-git-bot merged 2 commits into
OCA:18.0from
Tecnativa:18.0-FIX-web_company_color

Conversation

@CarlosRoca13

Copy link
Copy Markdown
Contributor

The company scss link was added inside the methods whose result is stored by ormcache (`_generate_asset_links_cache` / `_generate_asset_links`). The cache key does not depend on the company, and the override changed the cached list in place with `+=`. Each request added its company scss to the shared list, so the colors shown were those of the last company added in that worker.

Now the company scss is added in `_get_asset_links`, which is not cached, and a new list is returned without changing the cached one.
@pedrobaeza pedrobaeza added this to the 18.0 milestone Oct 27, 2026
@pedrobaeza

Copy link
Copy Markdown
Member

Please check CI

@CarlosRoca13
CarlosRoca13 force-pushed the 18.0-FIX-web_company_color branch 2 times, most recently from d2e088f to 594cca7 Compare October 27, 2026 14:26
@CarlosRoca13

Copy link
Copy Markdown
Contributor Author

Solved @pedrobaeza

@pedrobaeza pedrobaeza left a comment

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.

It seems you have nailed the problem!

Tested and so far no glitches found.

@ljmnoonan

Copy link
Copy Markdown
Member

Hello, this still breaks dark mode, which is expected. of course, but would be nice to fix eventually.

One note that does not concern dark mode: the current app name has dropdown-item class, and so gets the "Submenu Text Color". Would it not be better to style that particular item to "Navbar Text Color"?

Company colors are likely to be solid, darker colors, requiring a light text color, but the dropdown remains light. This forces us to choose which element we want to contrast with:
image
image
image

@CarlosRoca13
CarlosRoca13 force-pushed the 18.0-FIX-web_company_color branch from 594cca7 to 8249715 Compare October 28, 2026 10:29
@CarlosRoca13

Copy link
Copy Markdown
Contributor Author

Hi @ljmnoonan can you check if it correct now? Thanks 😄

@ljmnoonan

Copy link
Copy Markdown
Member

Text is good now, but it has introduced another slight problem:
Normal odoo does not darken the background of the app name on hover, and for good reason too, because the text is not centered on the background. Instead, it just lightens the text a bit on hover.
This is now darkening the background, so the uncentered text looks a bit odd.

2026-10-28.06-50-32.mp4

While doing this, btw, I noticed another small inconsistency which is not related to your changes. Changing the colors and saving triggers a page reload, but resetting them does not.

@CarlosRoca13
CarlosRoca13 force-pushed the 18.0-FIX-web_company_color branch from 8249715 to e7bf1d6 Compare October 28, 2026 11:26
@CarlosRoca13

Copy link
Copy Markdown
Contributor Author

Solved both problems 😄

@ljmnoonan

Copy link
Copy Markdown
Member

Very nice!
One problem remains; the home menu button is getting a dark hover background and is also off center.
Just to be clear, I do not use this module; I only know that it causes problems for users with web_dark_mode which I use and maintain as much as possible. That is why I came looking at this PR. I noticed some problems and decided to point them out rather than just move along.
So I hope you understand I am not trying to waste your time nitpicking on tiny UI issues :)
Thank you!

@pedrobaeza

Copy link
Copy Markdown
Member

/ocabot merge patch

Please fw-port it

@OCA-git-bot

Copy link
Copy Markdown
Contributor

Hey, thanks for contributing! Proceeding to merge this for you.
Prepared branch 18.0-ocabot-merge-pr-3759-by-pedrobaeza-bump-patch, awaiting test results.

@OCA-git-bot
OCA-git-bot merged commit 6d408ce into OCA:18.0 Oct 28, 2026
9 checks passed
@OCA-git-bot

Copy link
Copy Markdown
Contributor

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

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.

6 participants