-
-
Notifications
You must be signed in to change notification settings - Fork 2k
[18.0][IMP] web_theme_classic Add dark mode and some cleanup #3322
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: 18.0
Are you sure you want to change the base?
Conversation
|
Hi @legalsylvain, |
legalsylvain
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thanks for this refactoring and improvment !
|
Hi @ljmnoonan thanks for this PR, I started looking at how to improve borders on monetary fields and realized you did it as well ! Would you mind trying to also fix the currency symbol which is not aligned with amount : While testing, I found out that adding align-self: center;does the trick (although Firefox reports that it has no effect) |
f203db2 to
bfe3fe8
Compare
|
@remi-filament, thank you very much for the heads up! Screenshots:
|
bfe3fe8 to
acb7550
Compare
remi-filament
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great, thanks @ljmnoonan it is much better and I think current limitations are accceptable (were already there before I guess) !
Maybe it is worth adding them in README in ROADMAP section ?
|
This PR has the |
|
Hi. ready to merge ? regards |
|
@legalsylvain |
Refactor all selectors to mirror the way odoo sets them. This results in a MUCH broader application of our styles that is not limited to form views. Prefix all vars with "wtc-" in order to prevent them from overriding or being overridden by bootstrap vars. Suffix all vars with !default to ensure that they can be overriden by dark mode. Add $wtc-input-color-required to ensure text contrasts on $wtc-input-background-color-required. This is especially necessary in instances like the quantity and unit price columns of a sale order because by default these fields are set to a light blue. Remove border on .note-editable, i.e. the HTML editor. This feature was broken and, as far as I am concerned, unnecessary. Remove all button customization as it was all broken and is superceeded by OCA/web/web_save_discard_button anyways.
web_dark_mode is not merged yet. OCA#3324
acb7550 to
5f2789c
Compare
|
@legalsylvain @remi-filament Having already wasted enough time on it, I went all in and adjusted the whole thing to follow odoo's example, and I believe the result is much more comprehensive. Please let me know what you think! PS: I will update the PR description and screenshots to reflect the new state of things in order to avoid confusion in the future |






This could be 2 separate PRs, so please let me know if you would like me to split them.
The original purpose of this PR was to make this module play with web_dark_mode as promised in my previous PR, #3324. This is implemented in 5f2789c.
Here are some screenshots of this module in use in conjunction with web_dark_mode:

Before:
After:


However, while implementing the dark mode assets, I discovered a whole bunch of broken features and deficiencies in the existing code, so I decided to go about fixing them. All the changes are detailed in the commit message on a3e37ed, but I will attach some before and after pictures here for clarity.
Add $wtc-input-color-required
Before:

After:


Add CSS selectors to consistently handle many2many fields added as a custom property
Before:

After:


Fix monetary field selectors.
Before:


After:


Please let me know if you would like any changes!