[FIX] website, web_editor: review "small" font-size behavior

Before this commit, since [1], the "small" class behavior of Bootstrap
was changed to use a fixed value instead of being dependent on the
context (using `em` units).

This was a bad choice, given the fact that the "small" class could
be used in the past with its previous behavior in custo but also in
default Odoo layouts, where it could be use in more legit cases that the
one Odoo currently offers: instead of applying the class to a whole
paragraph, applying it to part of a title.
In that case, since [1], we can have a big title ... with a very small
text next to it, which may be strange. Of course, the proper way to
achieve a big title with a smaller text next to it would be to not use
"small" but another hx font-size, but the legit bootstrap behavior which
is to use their "small" class is thus broken.

This commit keeps the current possibilities (big title with very small
text next to it) but does it by using new custom Odoo classes instead of
changing the Bootstrap "small" one. That way, legit custo in previous
versions (or trying to use default Bootstrap in this version) will be
supported.

Note that this commit will impact existing 17.0 users though: if they
actually configured a big title with small inner text... those will
become big title with slightly smaller inner text (the default Bootstrap
behavior). Worse: a big title whose size was reduced using the font-size
selector and choosing "small" will now become a big title. We think this
is worth the risk (see PR description for more visual details).

However, notice that "legit" use case of the "small" font-size in 17.0
will be kept untouched: e.g. adding a small text on its own or next to
a paragraph: the "smaller" Bootstrap behavior is now computed based on
the ratio of the configured base and small font-sizes.

Note that some additional "bugs" were found investigating this:
- Form descriptions use the "small" class for no reason and should
  probably not be possible to customize ("font style"-wise) anyway.
- The use of the "small" *tag* should probably be reviewed in all Odoo
  layouts and/or its interaction with the editor be fixed.

[1]: https://github.com/odoo/odoo/commit/194f73a9bbad8c3c3fb5c378e7bdfa704aaacdc0

closes odoo/odoo#149590

Signed-off-by: Romain Derie (rde) <rde@odoo.com>
This commit is contained in:
qsm-odoo
2024-01-29 12:05:12 +00:00
parent ac62af7e25
commit 7931d1a14a
11 changed files with 80 additions and 16 deletions
@@ -3172,7 +3172,10 @@ export class OdooEditor extends EventTarget {
const block = closestBlock(sel.anchorNode);
let activeLabel = undefined;
for (const [style, cssSelector, isList] of [
['paragraph', 'p:not(.small, .lead)', false],
// TODO we might want to review this list to not mention o_xxx
// classes but be a setting instead? Probably after current
// refactorings being made in master.
['paragraph', 'p:not(.small, .lead, .o_small)', false],
['pre', 'pre', false],
['heading1', 'h1:not(.display-1, .display-2, .display-3, .display-4)', false],
['heading2', 'h2', false],
@@ -3185,7 +3188,10 @@ export class OdooEditor extends EventTarget {
['display-3', 'h1.display-3', false],
['display-4', 'h1.display-4', false],
['blockquote', 'blockquote', false],
['small', '.small', false],
// Note: this button will apply the "o_small" class but as an
// approximation, we display "Small" if this actually use the
// Bootstrap "small" class.
['small', '.small, .o_small', false],
['light', '.lead', false],
['unordered', 'UL', true],
['ordered', 'OL', true],
@@ -3247,7 +3253,7 @@ export class OdooEditor extends EventTarget {
const range = getDeepRange(this.editable, { sel, correctTripleClick: true });
const spansBlocks = [...range.commonAncestorContainer.childNodes].some(isBlock);
linkButton?.classList.toggle('d-none', spansBlocks || isInMedia);
// Hide link button group if it has no visible button.
const linkBtnGroup = this.toolbar.querySelector('#link.btn-group');
linkBtnGroup?.classList.toggle('d-none', !linkBtnGroup.querySelector('.btn:not(.d-none)'));
@@ -63,14 +63,22 @@ export const PROTECTED_BLOCK_TAG = ['TR','TD','TABLE','TBODY','UL','OL','LI'];
/**
* Array of all the classes used by the editor to change the font size.
*
* Note: the Bootstrap "small" class is an exception, the editor does not allow
* to set it but it did in the past and we want to remove it when applying an
* override of the font-size.
*/
export const FONT_SIZE_CLASSES = ["display-1-fs", "display-2-fs", "display-3-fs", "display-4-fs", "h1-fs",
"h2-fs", "h3-fs", "h4-fs", "h5-fs", "h6-fs", "base-fs", "small"];
"h2-fs", "h3-fs", "h4-fs", "h5-fs", "h6-fs", "base-fs", "o_small-fs", "small"];
/**
* Array of all the classes used by the editor to change the text style.
*
* Note: the Bootstrap "small" class was actually part of "text style"
* configuration in the past... but also of the "font size" configuration (see
* FONT_SIZE_CLASSES). It should be mentioned here too.
*/
export const TEXT_STYLE_CLASSES = ["display-1", "display-2", "display-3", "display-4", "lead"];
export const TEXT_STYLE_CLASSES = ["display-1", "display-2", "display-3", "display-4", "lead", "o_small", "small"];
//------------------------------------------------------------------------------
// Position and sizes
@@ -84,3 +84,13 @@ $gray-900: map-get($grays, '900') !default;
$black: map-get($grays, 'black') !default;
$o-color-system-initialized: true;
// This was added by compatibility but it actually became a nice behavior: the
// bootstrap default "small" behavior will use the ratio of the configured base
// font size (if configured, e.g. with website settings) and the Odoo own's
// "small" font size. Grep: SMALLER_FONT_SIZE_RATIO.
$small-font-size: if(
variable-exists('font-size-base'),
($o-small-font-size / $font-size-base) * 1em,
null
) !default;
@@ -140,3 +140,9 @@ $o-we-auto-contrast-exclusions: () !default;
$colors: str-replace($colors, ' ', '%20');
@return $colors;
}
//------------------------------------------------------------------------------
// Fonts
//------------------------------------------------------------------------------
$o-small-font-size: 0.875rem !default;
@@ -90,7 +90,6 @@
@include print-variable('h5-font-size', $h5-font-size);
@include print-variable('h6-font-size', $h6-font-size);
@include print-variable('font-size-base', $font-size-base);
@include print-variable('small-font-size', $small-font-size);
}
html, body {
@@ -250,6 +249,19 @@ img.ms-auto, img.mx-auto {
width: auto;
}
%o-small-font-size {
@include font-size($o-small-font-size);
}
// Dedicated class to be able to keep the default "small" behavior of bootstrap:
// being "smaller" that the context where it is used (em units). Here we want to
// define a specific fixed font-size for a smaller font-size than the base font
// size. Note that this class is designed to work as the display-x classes: an
// extra "styling" class to go on an element. For the "font-size class"
// equivalent, see o_small-fs below.
.o_small {
@extend %o-small-font-size;
}
@for $index from 1 through 4 {
.display-#{$index}-fs {
@include font-size(map-get($display-font-sizes, $index));
@@ -276,6 +288,12 @@ img.ms-auto, img.mx-auto {
.base-fs {
@include font-size($font-size-base);
}
// Equivalent "font-size" only for the Odoo own "o_small" class. Note that the
// "o_small" class currently also changes the font-size only but this is to stay
// consistent with the other classes which act that way (as display-x).
.o_small-fs {
@extend %o-small-font-size;
}
div.media_iframe_video {
margin: 0 auto;
+2 -2
View File
@@ -50,7 +50,7 @@
<a class="dropdown-item" href="#" id="light" data-call="setTag" data-arg1="p,lead" data-extended-text-style="">Light</a>
</li>
<li id="small-dropdown-item">
<a class="dropdown-item" href="#" id="small" data-call="setTag" data-arg1="p,small" data-extended-text-style="">Small</a>
<a class="dropdown-item" href="#" id="small" data-call="setTag" data-arg1="p,o_small" data-extended-text-style="">Small</a>
</li>
<li id="pre-dropdown-item">
<a class="dropdown-item" href="#" id="pre" data-call="setTag" data-arg1="pre">Code</a>
@@ -112,7 +112,7 @@
<li><a class="dropdown-item d-flex justify-content-between align-items-center" data-dynamic-value="h5-font-size" data-apply-class="h5-fs" href="#">? <span class="d-none o_we_font_size_badge badge rounded-pill text-bg-dark ms-4">Heading 5</span></a></li>
<li><a class="dropdown-item d-flex justify-content-between align-items-center" data-dynamic-value="h6-font-size" data-apply-class="h6-fs" href="#">? <span class="d-none o_we_font_size_badge badge rounded-pill text-bg-dark ms-4">Heading 6</span></a></li>
<li><a class="dropdown-item d-flex justify-content-between align-items-center" data-dynamic-value="font-size-base" data-apply-class="base-fs" href="#">? <span class="d-none o_we_font_size_badge badge rounded-pill text-bg-dark ms-4">Normal</span></a></li>
<li><a class="dropdown-item d-flex justify-content-between align-items-center" data-dynamic-value="small-font-size" data-apply-class="small" href="#">? <span class="d-none o_we_font_size_badge badge rounded-pill text-bg-dark ms-4">Small</span></a></li>
<li><a class="dropdown-item d-flex justify-content-between align-items-center" data-dynamic-value="small-font-size" data-apply-class="o_small-fs" href="#">? <span class="d-none o_we_font_size_badge badge rounded-pill text-bg-dark ms-4">Small</span></a></li>
</ul>
</div>
</t>
@@ -130,8 +130,6 @@ $display-font-sizes: (
6: o-website-value('display-6-font-size') or 2.5rem
) !default;
$small-font-size: o-website-value('small-font-size') !default;
// H2~H6 font families are custom variables.
$headings-font-family: $o-theme-headings-font !default;
$h2-font-family: $o-theme-h2-font !default;
@@ -1973,11 +1973,13 @@ $o-base-website-values-palette: (
'display-3-font-size': null, // Default to BS
'display-4-font-size': null, // Default to BS
// Forced to use `rem` by default instead of Bootstrap default using `em` as
// the editor would not be working with `em` values for now. This will have
// the drawback of changing the default small behavior of bootstrap which
// can normally be put inside titles etc... but that's not the way the
// editor will allow to use it for now. This is a compromise.
// Note that this custom value is not directly linked to the
// "small-font-size" variable of bootstrap. Indeed we want the default
// "small" behavior of bootstrap to stay the same: being "smaller" that the
// context where it is used (em units). Here we want to define a specific
// fixed font-size for a smaller font-size than the base font size. However,
// the default value for the "smaller" behavior of Bootstrap is based on
// this value anyway. See SMALLER_FONT_SIZE_RATIO.
'small-font-size': 0.875rem,
'google-fonts': null,
@@ -283,3 +283,5 @@ $o-theme-display-3-font: o-get-font-info('display-3') or $o-theme-headings-font
$o-theme-display-4-font: o-get-font-info('display-4') or $o-theme-headings-font !default;
$o-theme-navbar-font: o-get-font-info('navbar') or $o-theme-font !default;
$o-theme-buttons-font: o-get-font-info('buttons') or $o-theme-font !default;
$o-small-font-size: o-website-value('small-font-size') !default;
@@ -68,6 +68,16 @@
<t t-name="website.form_field_description">
<!-- The actual value for this case is handled in JS as it can be -->
<!-- edited with formatting by the user -->
<!--
TODO using "small" alongside with the "form-text" class is actually
a mistake in current Bootstrap version: it has no effect since the
"form-text" class already enforces the default "small" behavior of
Bootstrap. In master, remove "small" on all of these.
TODO changing "font style" (tag) should also be disabled on those
form descriptions as the behavior is kinda weird (but it mainly does
not make sense to try and change it over there anyway).
-->
<div t-if="default_description" class="s_website_form_field_description small form-text text-muted" contenteditable="true">
<t t-esc="default_description"/>
</div>
@@ -15,7 +15,7 @@ classNameInfo.set("h4-fs", {scssVariableName: "h4-font-size", start: 24, end: 34
classNameInfo.set("h5-fs", {scssVariableName: "h5-font-size", start: 20, end: 30});
classNameInfo.set("h6-fs", {scssVariableName: "h6-font-size", start: 16, end: 26});
classNameInfo.set("base-fs", {scssVariableName: "font-size-base", start: 16, end: 26});
classNameInfo.set("small", {scssVariableName: "small-font-size", start: 14, end: 24});
classNameInfo.set("o_small-fs", {scssVariableName: "small-font-size", start: 14, end: 24});
function checkComputedFontSize(fontSizeClass, stage) {
return {
@@ -99,6 +99,10 @@ function getAllFontSizesTestSteps() {
// That option is hidden by default because same value as base-fs
continue;
}
if (fontSizeClass === 'small') {
// There is nothing related to that class in the UI to test anymore.
continue;
}
steps.push(...getFontSizeTestSteps(fontSizeClass));
}
return steps;