[17.0] mig web_theme_classic

[legalsylvain]

Quite simple migration from 16.0 to 17.0

• add more specific css rule, using .o_form_view
• remove css rules for search view, because there are now native in Odoo.

[legalsylvain]

/ocabot migration web_theme_classic

[legalsylvain]

Hi @sbidoul. So I made my first PR on 17.0 branch.

• CI (pre-commit + test) looks OK 🆗
• generation of pyproject.toml file 🆗
• not generation of setup folder 🆗
• runboat : not available 👎

Note :
@yajo, @pedrobaeza, @sbidoul
as previously said here, I really think I'll dislike the fact that each commit comes with a modification of two files (readme.rst and static/description/index.html). This is generating useless extra diff, and make review harder. see extra noice here : 296da50

[sbidoul]

@legalsylvain runboat is up, now.

[francesco-ooops]

@sbidoul shouldn't readme files be converted in md for v17?

cc @TumbaoJu @lfreeke

[sbidoul]

Good point. Do you think we should automate that in pre-commit? Le mer. 8 nov. 2023, 11:54, francesco-ooops ***@***.***> a écrit :

…

@sbidoul <https://github.com/sbidoul> shouldn't readme files be converted in md for v17? cc @TumbaoJu <https://github.com/TumbaoJu> @lfreeke <https://github.com/lfreeke> — Reply to this email directly, view it on GitHub <#2660 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAFI623S7CQ3SETISIHAMHLYDNQHNAVCNFSM6AAAAAA7BJPFLCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQMBRGYZTGMBTGE> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[francesco-ooops]

@sbidoul yes, let's make the change happen :)

[francesco-ooops]

Screenshots should be updated. Also I think the button part has been addressed already:

[legalsylvain]

Done, thanks !

[sbidoul]

The PR to convert fragments to markdown in pre-commit is OCA/oca-addons-repo-template#228

[guestisp]

AWESOME !!! And working properly on v17.

Two small suggestions: lighter background (#f4f4f4 ?) for mandatory fields and a little bit of left/right padding on fields (ie 4px?)

From this:

To this:

[legalsylvain]

@legalsylvain runboat is up, now.

Thanks !

The PR to convert fragments to markdown in pre-commit is OCA/oca-addons-repo-template#228

thanks @sbidoul. works great.

hi @guestisp :
Thanks for your review.

Two small suggestions: lighter background (#f4f4f4 ?) for mandatory fields

I'm not sure. In a hand, I like your proposal, but in the other hand it is a matter of taste, and users have been accustomed to this color for many years. What do you think ? (I'm OK with moving in your direction.)

and a little bit of left/right padding on fields (ie 4px?)

Indeed. good proposal.

could you make a PR against my branch for the second point, (and eventually the first one ?)

Thanks !

[guestisp]

I'm not sure. In a hand, I like your proposal, but in the other hand it is a matter of taste, and users have been accustomed to this color for many years. What do you think ? (I'm OK with moving in your direction.)

The rationale is simple: 9 times to 10, grey fields are disabled. At first sight when i've installed this addon, i've thought that there grey fields was the disabled one in the form. But they are editable, and looking at the css class, they are required filed. So, a very light grey is better than a dark grey. But even better would be to change the color at all (light yellow? light green? light-something-but-not-grey?)

It's just my two cent as a fresh Odoo user, nothing more. A simple suggestion.

could you make a PR against my branch for the second point, (and eventually the first one ?)

Yup!

[guestisp]

what about white fields but with purple (odoo purple) borders for the required fields ?

[francesco-ooops]

@guestisp what about making it configurable?

[legalsylvain]

Hi

• regarding the required fields, it looks like there is no consensus, and make it configurable add a very big layer of complexity. So, I propose to keep it as it, as for the moment, the module copy the old behaviour of Odoo < 15.0.

• For the left/right padding on fields, I think it's consensual.

[guestisp]

@guestisp what about making it configurable?

that's not easy......

[guestisp]

there is an issue with dark mode. The grey fields on dark mode are .... terrible:

before doing a PR, do you know if there is a css class added somewere that i can use to detect the dark mode ? Because that gray has to be changed, in dark mode.

(if I saw properly, there isn't any class to use as selector, the css file is replaced totally, so, how can I check for dark mode inside the scss file?)

[legalsylvain]

before doing a PR, do you know if there is a css class added somewere that i can use to detect the dark mode ? Because that gray has to be changed, in dark mode.

I guess you talk about the EE module. If yes, I don't have access to EE, so I can't help you. But you can take a look on the code, if you can read it.

regards.

[guestisp]

not sure if dark mode is only available in Enterprise. i have to check.

and i have to find a way to set the background color variable only if not already defined, based on official docs [1], the scss for dark mode is loaded first, followed by everything else, so if the background was set in dark mode scss, following files should not redefine the var

[1] https://www.odoo.com/documentation/17.0/developer/reference/user_interface/scss_inheritance.html

[guestisp]

• For the left/right padding on fields, I think it's consensual.

grap#2

[remi-filament]

Thanks @legalsylvain for porting this module.
1 minor comment related to XML tag not necessary anymore in index.html raising a WARNING during installation.
Apart from this, 👍

[legalsylvain]

/ocabot rebase

@remi-filament : thanks for your patch. Is it now ok ?

thanks !

[celm1990]

LGTM tested functionally

[legalsylvain]

/ocabot merge nobump

[OCA-git-bot]

This PR looks fantastic, let's merge it!
Prepared branch 17.0-ocabot-merge-pr-2660-by-legalsylvain-bump-nobump, awaiting test results.

[OCA-git-bot]

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