[19.0][MIG] web_dark_mode: Migration to 19.0#3410
Conversation
Also, change asset bundle loading from 'prepend' to 'before' This is in line with Odoo's own way of doing it.
Also get rid of the bottom border.
If you go to any activity view and hover over an empty cell you will notice that without this fix the the on hover background of empty cells (.o_activity_empty_cell) set by $component-active-bg which is itself set as mix($o-action, $o-gray-100, 20%) through $o-component-active-bg completely ignores the overrides we set in primary_variables.dark.scss. This is because the assets for that view are lazy loaded, so we need to include our override bundles in the lazy bundle.
Just general customizations of BS variables to bring them in line with our style
Add secondary_variables override to get rid of colors that don't jive well with the dark background. Unfortunately this does introduce an issue where if you have many calendar users, you can run out of colors This issue exists in the enterprise dark mode as well, so hopefully it will be fixed.
Set the background color to be darker and tone down the border around it. The styling for the border is borrwed from the style of .o-mail-Composer-bg
Set attachment image background to white and fix the column background when hovering with a draggable card
…in forms It was being set to $o-form-lightsecondary which I set lighter, so this tones it down a bit.
The relevant changes to odoo code: odoo/odoo@2120e4c
1) Fix calendar events with color_0 which is used for private tasks, for example. Most noticable for private tasks in calendar view. 2) Adjust bootstrap bg-subtle and text-emphasis for info, success, warning, and danger. These are used for the epomnymous banners in html and in 19.0 for the purchase dashboard status cards. . Separate commit so that it can be backported to 18.0
|
/ocabot migration web_dark_mode I don't know if I can actually trigger this, but it's interesting to try :) |
|
Sorry @ljmnoonan you are not allowed to mark the addon to be migrated. To do so you must either have push permissions on the repository, or be a declared maintainer of all modified addons. If you wish to adopt an addon and become it's maintainer, open a pull request to add your GitHub login to the |
|
You can become the maintainer of the module for triggering that bot commands. /ocabot migration web_dark_mode |
|
@pedrobaeza, |
|
The PSC would be @OCA/web-maintainers |
|
@pedrobaeza |
ljmnoonan
force-pushed
the
19.0-mig-web_dark_mode
branch
from
fd61687 to
5ef5802
Compare
ljmnoonan
changed the title
web_dark_mode from OCA/web#3410
Previously, device dependent was strictly frontend and was set only if the color_scheme cookie had not already been set. This meant that it was useless unless you cleared your cookies. This fix implements two separete methods of detecting device preference: 1) Server side Adds request for Sec-CH-Prefers-Color-Scheme hint and uses it to serve up the correct asset bundle on the first load. This is mostly just for Chromium browsers. 2) Client side If Sec-CH-Prefers-Color-Scheme is None, client side js will check user.settings.dark_mode_device_dependent which, thanks to moving these prefs to res_users_settings is already loaded without a separate orm call, if true it will then check device preference and current cookie. If resetting the cookie is required, it will then trigger a reload. This is, of course, very bad, as it causes flicker and loads the assets twice, but thankfully it ony happens the first time you login or enable device dependent, or if you switch your device's system theme. This only applies to Firefox, Safari, or to Chromium browsers if the user has enabled anti fingerprinting settings. Sec-CH-Prefers-Color-Scheme docs are here: https://developer.mozilla.org/en-US/docs/Web/HTTP/Reference/Headers/Sec-CH-Prefers-Color-Scheme
ljmnoonan
force-pushed
the
19.0-mig-web_dark_mode
branch
from
5ef5802 to
993e2d3
Compare
alexey-pelykh
left a comment
alexey-pelykh
left a comment
There was a problem hiding this comment.
Thanks for this migration, Liam -- solid work overall.
The ir.http rework is well thought out. Moving from a cookie-only check to the color_scheme() method override aligns properly with how 19.0 decides which asset bundle to load. Using Sec-CH-Prefers-Color-Scheme for server-side device-dependent detection (with the JS fallback for Firefox/Safari) is a clean approach and nicely documented in the commit message.
Test coverage is thorough -- 9 test cases covering portal users, cookie/preference combinations, device-dependent mode, and the Vary/Accept-CH headers. The SCSS work is carefully scoped with good comments explaining the rationale behind individual overrides.
One minor thought (non-blocking): in _get_color_scheme, when user_device_dependant_scheme is True and neither browser header nor cookie mismatch applies, the method returns (None, existing_scheme_cookie). The caller color_scheme() then falls through to super().color_scheme(). This is correct behavior (lets the JS handle it), just wanted to confirm that was intentional -- and from the test_06 case and your commit message it clearly is.
LGTM.
|
@alexey-pelykh Hello, and thanks for the review. However, your review does appear to be AI generated and you have been asked already to stop this campaign of AI reviews. Please join the discussion in the contributors mailing list. On the comment. |
|
This PR has the |
|
/ocabot merge nobump |
|
Hey, thanks for contributing! Proceeding to merge this for you. |
|
Congratulations, your PR was merged at 977cf21. Thanks a lot for contributing to OCA. ❤️ |
|
Thank you @len-foss and @pedrobaeza ! |
13 participants
Odoo upped the contrast on a lot of things by making, for example, text
$gray-900instead of$gray-800. I did not dial this down as I did not think it was worth it to fight the trend and introduce hidden inconsistencies.This PR, besides migration, also includes a fix to the device dependent dark mode setting. I feel that this is justified seeing that the new logic is a direct consequence of changes in 19.0, namely, that the decision on which asset bundle to load is made not by checking the
color_schemecookie, but rather by calling thecolor_scheme()method inir_http. A full explanation of the new logic can be found in the last commit message.