[18.0][MIG] web_dark_mode: Migration to 18.0

[ljmnoonan]

This PR includes and supersedes #3267 with the permission of @len-foss.

Migrates web_dark_mode from 16 to 18 and overhauls the SCSS not only to look better, but also to correspond to Odoo's native structure as seen in the web module.

As with the previous version, its scope is limited to the backend, and so has no effect on POS or website.

Also, I have submitted #3322 to make web_theme_classic get along with this dark mode hope to do the same for web_responsive soon.

Looking forward to any feedback!

[mathben]

Thanks, it works for me.

Did you test the dashboard? The background is white.

[ljmnoonan]

Did you test the dashboard? The background is white.

The dashboard is, unfortunately, mostly canvas elements that I presume are rendered by JavaScript somewhere. I didn't look into it too much this time, but hopefully I'll be able to get it sorted later.
This problem existed in version 16 as well.

[len-foss]

FWIW, I have this in lambdao_web's ui.scss:

.app-menu-container {
  background-image: url(/lambdao_web/static/src/img/logo_grey.svg) !important;
  background-position: bottom right !important;
  background-repeat: no-repeat !important;
  background-size: 10% !important;
  background-color: #899499 !important;
}

and this in ui_dark.scss:

 .app-menu-container {
   background-color: #213330 !important;
 }

in any case, you don't need any JS to fix that. I had put a bunch of theme fixes for all the things I found egregious, this one being the worst offender :-)

[ljmnoonan]

@len-foss
Thank you for pointing that out! I'll try to get it fixed.

[ljmnoonan]

@len-foss, app-menu-container class is from web_responsive, the app switcher dashboard as far as I can tell. I am working on a PR for a dark mode for web responsive too, but @mathben and I were referring to the dashboards module earlier.

Now, I am not very good with javascript, but it does seem that this is all being rendered in o_spreadsheet.js and the styles that are controlled by SCSS are deliberately set to stay in light mode even when dark mode is active in o_spreadsheet_extended.dark.scss the first lines of which contain a comment mentioning that odoo does not currently have an adequate toolchain to support darkmode in spreadsheet.
I am not going to pursue this any further at the moment as it looks like a ton of work, and something that should actually be contributed upstream to odoo anyways.

[len-foss]

Oh sorry for the confusion. After the previous message I saw that I had a bunch of "fixes" to the dark mode, but without proper doc on chat it was. I agree with your analysis 👍

[mathben]

Can you review please @legalsylvain @yajo @LoisRForgeFlow
Thanks you!

[ljmnoonan]

@len-foss, would you mind reviewing this so we could maybe get some movement?
Thank you!

[len-foss]

LGTM
In any case, I don't see any good reason to stall the PR, except demoralization of potential contributors.

For completeness, there are some issues left (I guess left from the implementation of dark mode in CE that is severely lacking) -- checking on the runboat, I could see the following, by example.

[pedrobaeza]

For completeness, there are some issues left (I guess left from the implementation of dark mode in CE that is severely lacking) -- checking on the runboat, I could see the following, by example.

I think that's because web_classic_theme. Please uninstall it. I see that module as very harmless, as it's very bad for the visual health and ergonomics.

[ljmnoonan]

@len-foss and @pedrobaeza
I fixed the styles causing that eyesore in web_theme_classic in #3322. Would you like me to rebase so this becomes visible in runboat?
Thanks for reviewing!

[ljmnoonan]

I see that module as very harmless, as it's very bad for the visual health and ergonomics.

Also, btw, I have #3394 open to make web_theme_classic a user or even device option, so it isn't do all or nothing

[pedrobaeza]

No need, let's move with this:

/ocabot migration web_dark_mode

/ocabot merge nobump

@len-foss the problem about merging without proper reviewing from trusted reviewers is to lower the quality perception of people using OCA modules. I try to push some of them when others that are more or less familiar do reviews, but there's also a trend of some people "spitting" migration PRs (or even imp) not being even properly tested in their end.

[OCA-git-bot]

What a great day to merge this nice PR. Let's do it!
Prepared branch 18.0-ocabot-merge-pr-3324-by-pedrobaeza-bump-nobump, awaiting test results.

[OCA-git-bot]

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

[pedrobaeza]

@ljmnoonan but are you using web_classic_theme in your deployments? As said, I think keeping such purple background and hard borders is very harmless for the view health. I can agree default theme may be too "light" in some parts, but we can work on that not on that extreme way. Even that changes can be included in web_responsive, and to have several themes on it (the system is already prepared for that).

[OCA-git-bot]

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

[len-foss]

@ljmnoonan oh, sorry for the noise again -- I just hopped on runboat without much more checks (and I was indeed confused as to how such an obvious issue could have been overlooked). Thanks for your work!

[ljmnoonan]

@pedrobaeza, I don't have any real production deployments yet as I am just learning in my free time with the goal of eventually setting this up at the place I work (a lumber yard). I do like having web_theme_classic available though, but I also like to be able to turn it off. Really, it is most helpful for custom properties in tasks and such. I did a lot of cleanup on it in my earlier PR too, so it now it overrides the relevant css at the same level it was defined at, rather than the old approach which "chased" specific fields in specific contexts, resulting in inconsistencies and conflicts.

A screenshot with the new style

As for the idea of moving web_dark_mode to web_responsive, I totally agree that it would make a lot of sense. However, as things are right now in web_responsive the "themes", which only actually affect the app switcher, are implemented in a rather convoluted way using both scss and css variables switching based on an HTML data attribute rather than odoo's native backend asset bundling logic specifically built for dark mode and for which many core odoo apps have dedicated asset bundles. This is something I would like to discuss and work on, so I opened #3387 , but have so far got no input.

Thank you for the approval on this PR too!

[pedrobaeza]

@ljmnoonan that's at least less aggressive, but I suppose that only in dark mode. What I mean is to touch the light part in web_responsive for not requiring web_classic_theme at all. Some things should be sacrificed, but finding a good compromise between ergonomics and user awareness.

And congratulations for your work.

[ljmnoonan]

@pedrobaeza Ah, I see what you mean; integrate dome of the functionality of web_theme_classic into web_responsive. Maybe just bottom borders? Maybe revert to default in list view too.