[16.0][ADD] Module web_dark_mode #2351

fkantelberg · 2022-11-29T17:49:58Z

Odoo 16.0 allows an easier implementation of a dark mode and I gave it a try. I want to contribute my work.

I appreciate it if you have hints about the color scheme or find weird/bad looking places.

legalsylvain

hi @fkantelberg. Thanks for sharing !

Functional Test OK. (no code review)
One minor point thought :
-> go to settings : the background is very light, don't you think ?

could you add a text in a ROADMAP.rst file to mention that the module does'nt work for the point of sale module and requires extra glue modules ?
could you take a look on pre-commit errors ?

thank !

etobella · 2022-11-29T20:40:51Z

Interesting module. I made similar module from my side as a test but with a different approach with odoo (in order to avoid similarities)

I also kept the the colors on the top bar in order to allow the dark mode to work with other possible modules like web_company_color

2022-11-27.20-57-39.mp4

I understand that you made it before, so it makes sense to keep your module.

etobella · 2022-11-29T21:15:03Z

If you are interested, you can check it here:

https://github.com/tegin/web/tree/16.0-add-web_dark/web_dark

BTW, the error @legalsylvain found is fixed there

ioans73

Functional review 👍🏻

fkantelberg · 2022-11-30T07:35:12Z

Thank you for your response.

@legalsylvain Yes the settings page was light (again). This shows how much 16.0 still changes. I made it dark again and added a ROADMAP. With the new way of the assets it's maybe possible without a glue module otherwise a glue module might be wise.

The pre-commit seems a general error with loading flake8 from gitlab instead of github. Not sure how to tackle that because it's an issue in the entire CI of the OCA.

@etobella
Thank you. We can work together to make Odoo not blinding. I guess the web_company_color might still work because !important is used there. I haven't checked it yet because web_company_color isn't migrated yet. One main thing I wanted to solve is that the setting is device-independent :)

ap-wtioit · 2022-11-30T07:39:19Z

@fkantelberg the issue with flake8 was solved in OCA/oca-addons-repo-template#170, now it should be fixed if you run copier -f update like pedro mentioned here OCA/oca-addons-repo-template@a48f386

etobella · 2022-11-30T07:43:31Z

Well, to make it device independent, you should use only cookies, not a configuration on a user. The code used to make the cookie change without using the user was the following:
https://github.com/tegin/web/blob/16.0-add-web_dark/web_dark/static/src/js/dark_mode.esm.js#L13-L15

BTW, do you want to prefer the button on the user menu or something like the moon/sun button?

pedrobaeza · 2022-11-30T07:43:33Z

Let me fix the pre-commit in a separate PR. Then you can rebase.

fkantelberg · 2022-11-30T07:50:10Z

Well, to make it device independent, you should use only cookies, not a configuration on a user. The code used to make the cookie change without using the user was the following: https://github.com/tegin/web/blob/16.0-add-web_dark/web_dark/static/src/js/dark_mode.esm.js#L13-L15

BTW, do you want to prefer the button on the user menu or something like the moon/sun button?

You read me wrong. I wanted to make it device independent (not only cookie in browser). The user doesn't have to configure it on every device which would be the case if it's only a cookie.

I prefer that you need 2 clicks to change it. I love the sun/moon from your module but it's in a corner where many things to click are and it prevents some unintentional changes that way.

pedrobaeza · 2022-11-30T07:50:29Z

The fix is merged. Now you can rebase and remove later commit. Anyway, you need to make the following changes:

Rename JS files to .esm.js for proper linting.
Execute pre-commit run -a in local before committing or install the hook (pre-commit install -f) for being executed automatically on each commit.

etobella · 2022-11-30T08:01:01Z

You read me wrong. I wanted to make it device independent (not only cookie in browser). The user doesn't have to configure it on every device which would be the case if it's only a cookie.

I think it is not bad to have different configuratons according to your device, as this is more flexible. When accessing from my computer I might prefer dark mode and on my mobile I might prefer light mode. The best option would be to use by default the configuration of your browser 🤔

I prefer that you need 2 clicks to change it. I love the sun/moon from your module but it's in a corner where many things to click are and it prevents some unintentional changes that way.

Good point 😄

fkantelberg · 2022-11-30T08:28:18Z

You read me wrong. I wanted to make it device independent (not only cookie in browser). The user doesn't have to configure it on every device which would be the case if it's only a cookie.

I think it is not bad to have different configuratons according to your device, as this is more flexible. When accessing from my computer I might prefer dark mode and on my mobile I might prefer light mode. The best option would be to use by default the configuration of your browser thinking

Is that an option for the user or a global one?

jcdrubay

I did some functional tests on Runboat.
It looks great.
One thing that could be improved is to "auto-save" changes in progress when switching modes.

etobella · 2022-11-30T10:02:13Z

You read me wrong. I wanted to make it device independent (not only cookie in browser). The user doesn't have to configure it on every device which would be the case if it's only a cookie.

I think it is not bad to have different configuratons according to your device, as this is more flexible. When accessing from my computer I might prefer dark mode and on my mobile I might prefer light mode. The best option would be to use by default the configuration of your browser thinking

Is that an option for the user or a global one?

This is something defined on the browser, however, I am not sure if this could work properly, because some changes might be needed from odoo side... It is more a wish than an option 😭

Anyway, I am not sure that we must define this by user, I would prefer to keep that on a cookie, in the same way Odoo does in order to follow the same logic with Odoo EE. If we want to force a cookie by user, I think it might be better to define a field on the user and accessing it from Preferences. On login, if the cookie is not already present, it will be defined as the value of the user. WDYT?

fkantelberg · 2022-11-30T17:18:30Z

This is something defined on the browser, however, I am not sure if this could work properly, because some changes might be needed from odoo side... It is more a wish than an option sob

Anyway, I am not sure that we must define this by user, I would prefer to keep that on a cookie, in the same way Odoo does in order to follow the same logic with Odoo EE. If we want to force a cookie by user, I think it might be better to define a field on the user and accessing it from Preferences. On login, if the cookie is not already present, it will be defined as the value of the user. WDYT?

I added a configuration to the user preferences to make the dark mode device dependent if the user wants it. Default stays device independent. :)

etobella · 2022-12-01T11:33:09Z

Thanks!

AntoniRomera · 2022-12-02T14:18:56Z

I see super well this configuration but I have a doubt, the mobile devices along with several operating systems including windows 11, has settings for dark / light mode, would not it be better to take the configuration of the same device.

OSevangelist · 2022-12-02T14:29:47Z

Hi @etobella @pedrobaeza @legalsylvain @jcdrubay @ap-wtioit @ioans73

Interesting module. I made similar module from my side as a test but with a different approach with odoo (in order to avoid similarities)

@fkantelberg and me discussed that "similarity issue" very critically, taking into account also the other side of the potential spectrum of thoughts. We did so with regard to what recently happened in OCA/spreadsheet#1 , how this case was / wasn't handled and especially with the fact that a "dark mode" generally helps to keep the eyes healthy and reduce a waste of resources adding up to our all goal to improve sustainability and has therefore become a standard function of such web based systems.

Overall, we came to the conclusion that the approach is reasonably different (code wise) from the EE functionality, as the later does much more (i.e. higher functional coverage / complexity) is not encapsulated in a single module (different way of implementation) and doesn't have some of the features @fkantelberg added here (even replying to your request already).

The colors / coloring schemes and shades can be considered industry best practices and are well described in various public outlets (e.g. https://arpit-batri.medium.com/dark-mode-ui-conversion-604d2ecc0083).

However, if you have arguments / concerns that it is not different enough from the enterprise functionality please share these arguments / concerns here and lets all together try to set a benchmark of "difference" ultimately by also asking the other side of the spectrum for its consent as we can still change the function / coverage / implementation details further if necessary.

I am honestly sick of reinventing wheels especially for functions that are not economically relevant (i.e. nobody buys a license for a dark mode function) and i know so do many others in the community. We need to establish a common understanding with the vendor on what is possible and what not. Nobody wants to infringe anyones IPR. So again please share your thoughts on the matter and as soon as the case is being maturely discussed here i am happy to ask the Chief Executive Officer of the vendor for his consent or change requests.

pedrobaeza · 2022-12-02T14:52:48Z

Odoo S.A. doesn't have the exclusivity of an enterprise feature. We can develop an exact drop-in replacement. The only condition is to not look at their code to develop it. But given that some of the tools used for the feature are at community level, it's very likely that the result code will be similar.

I don't think we should censor ourselves by fear of being very similar to enterprise.

AntoniRomera · 2022-12-02T15:10:19Z

web_dark_mode/static/src/js/switch_item.esm.js

+        registry.category("user_menuitems").add("darkmode", darkModeSwitchItem);
+        if (!cookie.current.color_scheme) {
+            const data = await orm.webSearchRead(
+                "res.users",
+                [["id", "=", user.userId]],
+                ["dark_mode"]
+            );
+
+            const dark_mode = data.length && data.records[0].dark_mode;
+            cookie.setCookie("color_scheme", dark_mode ? "dark" : "light");
+        }


window.matchMedia('(prefers-color-scheme: dark)').addEventListener('change', event => { cookie.setCookie("color_scheme", event.matches ? "dark" : "light"); browser.location.reload(); }) const dark_mode = window.matchMedia('(prefers-color-scheme: dark)').matches; cookie.setCookie("color_scheme", dark_mode ? "dark" : "light");

Interesting snippet to use the browser or system defaults. It would reload the entire page twice if your preferred scheme is dark which can be annoying but endurable for backend I guess. I'm just unsure if this is preferred because when it comes to workstations your system preference is just a default and Odoo is used in these environments (e.g. POS, non-personal devices for inventory etc)

If we are talking about work environments, it can happen the same as you activate the dark mode or disable it if the equipment you use is not unipersonal, but I'm not saying to remove the on or off button, sorry if I did not explain, it would only be interesting that the default is the one on the device

As far as I can see it can only happen in device dependent mode that the cookie isn't set by python code. In that case the dark_mode flag of res.users is just the last value selected on any device. In that case using the system preferences might be the better default. I'll give it a try. 👍

Alright the JS uses the prefers-color-scheme as default now if you have activated the device dependent mode.

etobella · 2022-12-02T15:20:20Z

IMO there is no issue with EE as the approach and functionality are different. Also, you did not took any EE code, so you should keep on. We cannot be scared as we did nothing wrong here.

It is a great addition for community, as this is needed by everyone.

OSevangelist · 2022-12-02T18:53:22Z

Thanks @pedrobaeza @etobella for your feedback. I think even looking at the EE code would be fine in general but the drop-in replacement must be developed in sufficiently different way. Of course, sometimes even two absolutely independent 'agents' would develop things in the same / almost similar way, because the framework and its constraints forces them to do so. If this is the case the functionality as a whole is not considered to have much technical depth and that is a very significant argument on whether something can be protected by copyright law or not. So i do agree with you. Lets merge it then!

OCA-git-bot · 2022-12-04T18:00:02Z

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

OSevangelist · 2022-12-07T08:09:41Z

@etobella you see to be among the eligable persons on the web repo. Would you mind to initiate the ocabot to merge this piece. Of course if anyone of the other PSC members is faster, equally welcome ;-) Thx a lot!

pedrobaeza · 2022-12-07T08:39:52Z

/ocabot merge nobump

OCA-git-bot · 2022-12-07T08:40:04Z

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

OCA-git-bot · 2022-12-07T08:43:56Z

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

OSevangelist · 2022-12-07T09:49:17Z

thanks a lot @pedrobaeza

legalsylvain approved these changes Nov 29, 2022

View reviewed changes

ioans73 approved these changes Nov 30, 2022

View reviewed changes

OCA-git-bot added the approved label Nov 30, 2022

etobella closed this Nov 30, 2022

etobella reopened this Nov 30, 2022

fkantelberg force-pushed the 16.0-web_dark_mode branch from d3ea694 to 956f95a Compare November 30, 2022 08:03

jcdrubay reviewed Nov 30, 2022

View reviewed changes

AntoniRomera reviewed Dec 2, 2022

View reviewed changes

OCA-git-bot added the ready to merge label Dec 4, 2022

fkantelberg force-pushed the 16.0-web_dark_mode branch from 5d56d6d to 1fb3cd3 Compare December 5, 2022 18:04

[ADD][16.0] Module web_dark_mode

abe28c7

fkantelberg force-pushed the 16.0-web_dark_mode branch from 1fb3cd3 to abe28c7 Compare December 5, 2022 18:06

pedrobaeza added this to the 16.0 milestone Dec 7, 2022

OCA-git-bot added the bot is merging ⏳ label Dec 7, 2022

OCA-git-bot merged commit 11a649d into OCA:16.0 Dec 7, 2022

OCA-git-bot added merged 🎉 and removed bot is merging ⏳ labels Dec 7, 2022

legalsylvain mentioned this pull request Jan 18, 2023

[ADD] account_reconcile_oca OCA/account-reconcile#500

Merged

25 tasks

legalsylvain mentioned this pull request Feb 3, 2023

[16.0] Error when uninstalling web_dark_mode #2402

Closed

fkantelberg deleted the 16.0-web_dark_mode branch February 6, 2023 07:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[16.0][ADD] Module web_dark_mode #2351

[16.0][ADD] Module web_dark_mode #2351

fkantelberg commented Nov 29, 2022

legalsylvain left a comment •

edited

Loading

etobella commented Nov 29, 2022

etobella commented Nov 29, 2022

ioans73 left a comment

fkantelberg commented Nov 30, 2022

ap-wtioit commented Nov 30, 2022

etobella commented Nov 30, 2022

pedrobaeza commented Nov 30, 2022

fkantelberg commented Nov 30, 2022

pedrobaeza commented Nov 30, 2022

etobella commented Nov 30, 2022 •

edited

Loading

fkantelberg commented Nov 30, 2022

jcdrubay left a comment

etobella commented Nov 30, 2022

fkantelberg commented Nov 30, 2022

etobella commented Dec 1, 2022

AntoniRomera commented Dec 2, 2022

OSevangelist commented Dec 2, 2022

pedrobaeza commented Dec 2, 2022

AntoniRomera Dec 2, 2022 •

edited

Loading

fkantelberg Dec 5, 2022

AntoniRomera Dec 5, 2022

fkantelberg Dec 5, 2022

fkantelberg Dec 5, 2022

etobella commented Dec 2, 2022

OSevangelist commented Dec 2, 2022

OCA-git-bot commented Dec 4, 2022

OSevangelist commented Dec 7, 2022

pedrobaeza commented Dec 7, 2022

OCA-git-bot commented Dec 7, 2022

OCA-git-bot commented Dec 7, 2022

OSevangelist commented Dec 7, 2022

[16.0][ADD] Module web_dark_mode #2351

[16.0][ADD] Module web_dark_mode #2351

Conversation

fkantelberg commented Nov 29, 2022

legalsylvain left a comment • edited Loading

Choose a reason for hiding this comment

etobella commented Nov 29, 2022

etobella commented Nov 29, 2022

ioans73 left a comment

Choose a reason for hiding this comment

fkantelberg commented Nov 30, 2022

ap-wtioit commented Nov 30, 2022

etobella commented Nov 30, 2022

pedrobaeza commented Nov 30, 2022

fkantelberg commented Nov 30, 2022

pedrobaeza commented Nov 30, 2022

etobella commented Nov 30, 2022 • edited Loading

fkantelberg commented Nov 30, 2022

jcdrubay left a comment

Choose a reason for hiding this comment

etobella commented Nov 30, 2022

fkantelberg commented Nov 30, 2022

etobella commented Dec 1, 2022

AntoniRomera commented Dec 2, 2022

OSevangelist commented Dec 2, 2022

pedrobaeza commented Dec 2, 2022

AntoniRomera Dec 2, 2022 • edited Loading

Choose a reason for hiding this comment

fkantelberg Dec 5, 2022

Choose a reason for hiding this comment

AntoniRomera Dec 5, 2022

Choose a reason for hiding this comment

fkantelberg Dec 5, 2022

Choose a reason for hiding this comment

fkantelberg Dec 5, 2022

Choose a reason for hiding this comment

etobella commented Dec 2, 2022

OSevangelist commented Dec 2, 2022

OCA-git-bot commented Dec 4, 2022

OSevangelist commented Dec 7, 2022

pedrobaeza commented Dec 7, 2022

OCA-git-bot commented Dec 7, 2022

OCA-git-bot commented Dec 7, 2022

OSevangelist commented Dec 7, 2022

legalsylvain left a comment •

edited

Loading

etobella commented Nov 30, 2022 •

edited

Loading

AntoniRomera Dec 2, 2022 •

edited

Loading