Skip to content
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

[FW][FIX] website: fix some components in case of contrasted boxed layout #112136

Conversation

fw-bot
Copy link
Contributor

@fw-bot fw-bot commented Feb 7, 2023

[FIX] website, *: fix some components in case of contrasted boxed layout

*: website_slides

In some cases, components had dark text over dark background (or light
text over light background) by mistake.

Example:

  • Enter edit mode.
  • In the theme tab, choose "boxed" as page layout.
  • A color picker appears below to control the color behind the box.
  • Set it to a dark color (if your box main color is light)
  • Go to a course page (install website_slides)
  • Check the mobile version
    => The bootstrap tab and its section uses the dark color you set up as
    body color instead of the expected boxed layout color.

Another example:

  • Do the same thing (set up a dark color behind a boxed layout).
  • Go to a shop / product page.
    => The inputs are dark with dark text.

This is because of bootstrap which uses $body-bg as default value for
other variables, such as $nav-tabs-link-active-bg in the first case
described above. It also uses the variable in the creation of CSS rules
not controlled by explicit variables.

In 16.0, bootstrap was updated to 5.1.3 with 1 and this actually
increased the problem: input backgrounds now default to $body-bg,
amongst other things. Since 2, $body-bg is also used as the default
color for range thumbs.

In previous versions, this fix focused on fixing a critical component:
nav-tabs, for which the fix was straightforward.
Starting from 16.0, this commit will fix everything at the small risk of
changing the $body-bg variable meaning in the case of boxed layouts.
Before this commit, its meaning was "the color used for the background
behind the boxed layout (the background color)", so equal to the
Odoo value o-color('body'). After this commit, its meaning will be
"the color used for the background of the box itself", so equal to
o-color('o-cc1-bg'). The <body> background color will be forced by
using o-color('body') as the value for the related CSS variable
defined by bootstrap. This allows to have a correct CSS generation for
all components in case of boxed layouts: indeed, the components mix
their own color with $body-bg (or use it as it is) relying on the fact
this is the color which appears behind them... which was not right in
case of boxed layouts.

This commit actually fixes another bug that was found during adaptation.
It is 2-fold, and unfortunately, it does not make sense to fix one part
without the other as it would increase the problem without the other
part. The website_slides pages customize their default background color
to not be the one chosen by the user, but a mix of it with some
lightgray. Odoo default for the body being white, this makes it a
lightgray for website_slides pages. This is totally ok... but only in
"full" layout. In boxed layout, we have the 2-fold problem:

A. The mixed color is not applied to the boxed layout but on the
background behind the box. So if you have a white box above a black
background, in website_slides pages you won't have the black
background you expected to keep but a lighter version of it and the
website_slides box will not use the lightgray but stay white
(creating other inconsistencies as the lightgray would also be used
by other components like tabs, for that app only).

B. The mixed color is actually not mixing the right colors: it mixes
the hardcoded lightgray with the color of the background behind the
box, while it was intended to be the one of the content (the one of
the box), like in "full" layout.

The changes explained above about $body-bg naturally fixes (B). Not
fixing (A) at the same time would result in a big change for the color
which is behind the box. This commit fixes it at the same time by now
applying the color to the right element. In previous version, this could
be fixed as well but would require a different fix (not relying on
$body-bg). So it makes sense to merge this first and backport+adapt.

opw-3151962

Before After
image image
Before After
image image
Before After
image image
Before After
image image

Forward-Port-Of: #111780

@robodoo
Copy link
Contributor

robodoo commented Feb 7, 2023

@fw-bot
Copy link
Contributor Author

fw-bot commented Feb 7, 2023

@qsm-odoo @rdeodoo cherrypicking of pull request #111780 failed.

stdout:

Auto-merging addons/website/static/src/scss/bootstrap_overridden.scss
CONFLICT (content): Merge conflict in addons/website/static/src/scss/bootstrap_overridden.scss

stderr:

18:14:56.675594 git.c:455               trace: built-in: git cherry-pick e8cd294103c2a8ed49303d9eaa9bf2a42c2801c5
error: could not apply e8cd294103c... [FIX] website: fix some components in case of contrasted boxed layout
hint: After resolving the conflicts, mark them with
hint: "git add/rm <pathspec>", then run
hint: "git cherry-pick --continue".
hint: You can instead skip this commit with "git cherry-pick --skip".
hint: To abort and get back to the state before "git cherry-pick",
hint: run "git cherry-pick --abort".
----------
status:

Either perform the forward-port manually (and push to this branch, proceeding as usual) or close this PR (maybe?).

In the former case, you may want to edit this PR message as well.

@robodoo robodoo added forwardport This PR was created by @fw-bot conflict There was an error while creating this forward-port PR labels Feb 7, 2023
@C3POdoo C3POdoo added the OE the report is linked to a support ticket (opw-...) label Feb 7, 2023
*: website_slides

In some cases, components had dark text over dark background (or light
text over light background) by mistake.

Example:
- Enter edit mode.
- In the theme tab, choose "boxed" as page layout.
- A color picker appears below to control the color behind the box.
- Set it to a dark color (if your box main color is light)
- Go to a course page (install website_slides)
- Check the mobile version
=> The bootstrap tab and its section uses the dark color you set up as
   body color instead of the expected boxed layout color.

Another example:
- Do the same thing (set up a dark color behind a boxed layout).
- Go to a shop / product page.
=> The inputs are dark with dark text.

This is because of bootstrap which uses `$body-bg` as default value for
other variables, such as `$nav-tabs-link-active-bg` in the first case
described above. It also uses the variable in the creation of CSS rules
not controlled by explicit variables.

In 16.0, bootstrap was updated to 5.1.3 with [1] and this actually
increased the problem: input backgrounds now default to `$body-bg`,
amongst other things. Since [2], `$body-bg` is also used as the default
color for range thumbs.

In previous versions, this fix focused on fixing a critical component:
nav-tabs, for which the fix was straightforward.
Starting from 16.0, this commit will fix everything at the small risk of
changing the `$body-bg` variable meaning in the case of boxed layouts.
Before this commit, its meaning was "the color used for the background
behind the boxed layout (the <body> background color)", so equal to the
Odoo value `o-color('body')`. After this commit, its meaning will be
"the color used for the background of the box itself", so equal to
`o-color('o-cc1-bg')`. The `<body>` background color will be forced by
using `o-color('body')` as the value for the related *CSS* variable
defined by bootstrap. This allows to have a correct CSS generation for
all components in case of boxed layouts: indeed, the components mix
their own color with `$body-bg` (or use it as it is) relying on the fact
this is the color which appears behind them... which was not right in
case of boxed layouts.

This commit actually fixes another bug that was found during adaptation.
It is 2-fold, and unfortunately, it does not make sense to fix one part
without the other as it would increase the problem without the other
part. The website_slides pages customize their default background color
to not be the one chosen by the user, but a mix of it with some
lightgray. Odoo default for the body being white, this makes it a
lightgray for website_slides pages. This is totally ok... but only in
"full" layout. In boxed layout, we have the 2-fold problem:

A. The mixed color is not applied to the boxed layout but on the
   background behind the box. So if you have a white box above a black
   background, in website_slides pages you won't have the black
   background you expected to keep but a lighter version of it and the
   website_slides box will not use the lightgray but stay white
   (creating other inconsistencies as the lightgray would also be used
   by other components like tabs, for that app only).

B. The mixed color is actually not mixing the right colors: it mixes
   the hardcoded lightgray with the color of the background behind the
   box, while it was intended to be the one of the content (the one of
   the box), like in "full" layout.

The changes explained above about `$body-bg` naturally fixes (B). Not
fixing (A) at the same time would result in a big change for the color
which is behind the box. This commit fixes it at the same time by now
applying the color to the right element. In previous version, this could
be fixed as well but would require a different fix (not relying on
`$body-bg`). So it makes sense to merge this first and backport+adapt.

[1]: odoo@971e5a9
[2]: odoo@46e5387

opw-3151962

X-original-commit: 03f238a
@qsm-odoo qsm-odoo force-pushed the 16.0-16.0-opw-3166634-body-bg-bs5-qsm-8Hpy-fw branch from 115871d to 817d5ff Compare February 8, 2023 16:25
@C3POdoo C3POdoo requested review from a team February 8, 2023 16:27
Copy link
Contributor

@qsm-odoo qsm-odoo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@robodoo r+

Although this is a forward-port, this will require a partial backport again in community + enterprise, with additional fixes (although not the exact same to be a bit more stable over there). EDIT: 14.0/15.0 do not have the issue in appointment (other layouts) and website_slides boxed layout errors will be ignored in < 16.0 as not stable enough to fix (and most of the time ok or workaround).

@robodoo robodoo closed this in 14c985a Feb 8, 2023
@robodoo robodoo temporarily deployed to merge February 8, 2023 18:34 Inactive
@qsm-odoo qsm-odoo deleted the 16.0-16.0-opw-3166634-body-bg-bs5-qsm-8Hpy-fw branch February 9, 2023 09:36
StevenSermeus pushed a commit to acsone/odoo that referenced this pull request Mar 9, 2023
*: website_slides

In some cases, components had dark text over dark background (or light
text over light background) by mistake.

Example:
- Enter edit mode.
- In the theme tab, choose "boxed" as page layout.
- A color picker appears below to control the color behind the box.
- Set it to a dark color (if your box main color is light)
- Go to a course page (install website_slides)
- Check the mobile version
=> The bootstrap tab and its section uses the dark color you set up as
   body color instead of the expected boxed layout color.

Another example:
- Do the same thing (set up a dark color behind a boxed layout).
- Go to a shop / product page.
=> The inputs are dark with dark text.

This is because of bootstrap which uses `$body-bg` as default value for
other variables, such as `$nav-tabs-link-active-bg` in the first case
described above. It also uses the variable in the creation of CSS rules
not controlled by explicit variables.

In 16.0, bootstrap was updated to 5.1.3 with [1] and this actually
increased the problem: input backgrounds now default to `$body-bg`,
amongst other things. Since [2], `$body-bg` is also used as the default
color for range thumbs.

In previous versions, this fix focused on fixing a critical component:
nav-tabs, for which the fix was straightforward.
Starting from 16.0, this commit will fix everything at the small risk of
changing the `$body-bg` variable meaning in the case of boxed layouts.
Before this commit, its meaning was "the color used for the background
behind the boxed layout (the <body> background color)", so equal to the
Odoo value `o-color('body')`. After this commit, its meaning will be
"the color used for the background of the box itself", so equal to
`o-color('o-cc1-bg')`. The `<body>` background color will be forced by
using `o-color('body')` as the value for the related *CSS* variable
defined by bootstrap. This allows to have a correct CSS generation for
all components in case of boxed layouts: indeed, the components mix
their own color with `$body-bg` (or use it as it is) relying on the fact
this is the color which appears behind them... which was not right in
case of boxed layouts.

This commit actually fixes another bug that was found during adaptation.
It is 2-fold, and unfortunately, it does not make sense to fix one part
without the other as it would increase the problem without the other
part. The website_slides pages customize their default background color
to not be the one chosen by the user, but a mix of it with some
lightgray. Odoo default for the body being white, this makes it a
lightgray for website_slides pages. This is totally ok... but only in
"full" layout. In boxed layout, we have the 2-fold problem:

A. The mixed color is not applied to the boxed layout but on the
   background behind the box. So if you have a white box above a black
   background, in website_slides pages you won't have the black
   background you expected to keep but a lighter version of it and the
   website_slides box will not use the lightgray but stay white
   (creating other inconsistencies as the lightgray would also be used
   by other components like tabs, for that app only).

B. The mixed color is actually not mixing the right colors: it mixes
   the hardcoded lightgray with the color of the background behind the
   box, while it was intended to be the one of the content (the one of
   the box), like in "full" layout.

The changes explained above about `$body-bg` naturally fixes (B). Not
fixing (A) at the same time would result in a big change for the color
which is behind the box. This commit fixes it at the same time by now
applying the color to the right element. In previous version, this could
be fixed as well but would require a different fix (not relying on
`$body-bg`). So it makes sense to merge this first and backport+adapt.

[1]: odoo@971e5a9
[2]: odoo@46e5387

opw-3151962

closes odoo#112136

X-original-commit: 03f238a
Signed-off-by: Romain Derie (rde) <rde@odoo.com>
Signed-off-by: Quentin Smetz (qsm) <qsm@odoo.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
conflict There was an error while creating this forward-port PR forwardport This PR was created by @fw-bot OE the report is linked to a support ticket (opw-...)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants