ClayCSS: Support Document Font Size to be 62.5% or 10px for easier rem/em conversion

[pat270]

Issue from Slack Chat:

Hey, a question. Is there a reason for 1rem to be 16 and not 10px in Liferay 7.1 ga1 in the Classic Theme? (i know 16px is default but isn't 10px the best practice?)

you’d set font-size: 62.5%, then have 1rem map to 10px so the conversion is simpler, 2.4rem == 24px

Changing this outright in Atlas would break any theme currently extending it.

1. We could create a function that returns a rem value based on the document's base font size and a pixel value. We would have to wrap every rem value we use in that function. We would also have to overwrite all Bootstrap 4 variables that use rem values with this new pattern. I'm also not sure if this will work 100%. I've done a limited test and it's working but there are probably edge cases.

2. Another option is to provide a base 10px or 62.5% variable theme in the compatibility layer.

3. The final option I can think of is to offer a third party variable theme with base 10px or 62.5%.

All 3 of these options will only provide support for Sass inside our themes. Any Sass provided by our widgets and other plugins will have to be customized.

[sbaechler]

Clay already uses a Sass variable called $font-size-base. That variable should be set to the root font size. It is then possible to use this instead of 1rem without breaking the layout no matter what the base font size is.

Currently, there are still a few places in the styles where there is a hardcoded value of 1rem. This should be replaced with $font-size-base. A 0.8rem value can be replaced with $font-size-base * 0.8.

In liferay/liferay-js-themes-toolkit#44 I have noted some places that should be fixed.

[pat270]

Moving the discussion from #1332 (comment) so it's more visible.

Hi Patrick
Let me explain the idea behind this.
In fact, it does make a difference. $font-size-base is the size of the body text. This can now be decoupled from the rem value.

If you want to define 1rem as 10px while keeping a base font size of 16px you can do

$font-size-base: 1.6rem;
html {
font-size: 10px;
}
This is what you actually have to do now if you upgraded form a Liferay 7 site to 7.1 since Liferay 7 had a 10px root font size and 7.1 has 16px.

I don't think Bootstrap will change this variable. The implications would be too big. But if you prefer, we could have a $clay-font-size-base variable to separate the Liferay styles from the public UI.

I would also change the line widths to Pixel values. Otherwise you get fractions which don't render nicely.

Ok this is making sense now. We have some constraints we need to consider before we make a change like this:

1. We have Clay Base that extends Bootstrap which has hardcoded rem values based on 16px. We're currently unable to modify Bootstrap's variables directly and will need to import any variable changes before Bootstrap's variables.
2. Liferay CE/DXP has hardcoded rem values based on 16px that we can't easily convert. We have widget CSS that doesn't have access to Sass variables in the theme and can't scale using $clay-font-size-base.
3. There are themes based on Clay CSS that will break if we introduce this by default.

I'm still leaning toward an opt-in variable theme with base 10px since we will need to be importing variables anyways and this will minimize any breakages. We can implement @sbaechler 's suggestion by setting $font-size-base: 1.6rem and propagating it to all the component variables. It will look something like:

_clay-base-theme-with-base-10px.scss

// Setting to enable changing the document's base font size
$enable-base-10: false !default;

@if ($enable-base-10) {
	// The font size on the html element
	$document-font-size: 10px !default;

	// This is the base font size used for all component variables
	$clay-font-size-base: 1.6rem !default; // 16px

	// This sets font size on body element
	$font-size-base: 1.6rem !default; // 16px

	$border-radius: ($clay-font-size-base * 0.25) !default; // 4px
	$border-radius-lg: ($clay-font-size-base * 0.375) !default; // 6px
	$border-radius-sm: ($clay-font-size-base * 0.1875) !default; // 3px

	$btn-padding-x: ($clay-font-size-base * 0.75) !default; // 12px
	$btn-padding-y: ($clay-font-size-base * 0.375) !default; // 6px

	$btn-padding-x-sm: ($clay-font-size-base * 0.5) !default; // 8px
	$btn-padding-y-sm: ($clay-font-size-base * 0.75) !default; // 12px

	$btn-padding-x-lg: $clay-font-size-base !default; // 20px
	$btn-padding-y-lg: ($clay-font-size-base * 0.625) !default; // 10px
}

This seems a bit overboard honestly, but I can see a use case where someone picked an arbitrary font size value for the document and built their theme around it. If we decide to go forward with this, it would only apply partially to theme CSS in CE/DXP.

The CSS for the Product Menu and Control Menu are included in the document as a separate stylesheet and it doesn't have access to any of the Clay CSS Sass variables. This idea also applies to widget specific CSS. It would be up to you guys to overwrite any extraneous theme / module specific CSS yourself.

This could be more or less work depending on your use case. There is also a site that is a WIP https://v2-claypaver.wedeploy.io/. We are planning to add a px to rem converter that could make it easier to convert values no matter the document base font size.

[sbaechler]

About those constraints, I just checked the Bootstrap source for hardcoded rem variables and I only found 2 locations: One is the margin above the close button in the alert widget and another one is in the forms padding. The forms are highly customised by Liferay anyways so this should not be an issue.

I have been using Bootstrap 4 based themes with 10px base font size without any problems for years now.

Current themes should not break if $font-size-base is set to 1rem. However, they are already breaking now because the base font size has changed from 10px to 16px with the 7.1 update.

I don't really know what to say about the hardcoded widgets that cannot be overriden. I assume they are in the same Bootstrap 4 namespace as the rest of the site. This basically forces everyone to rewrite their themes completely when they upgrade from Liferay 7.0 to 7.1.

There is also the bug that the form plugin overrides the custom form styles with the default ones.

I am not convinced about the benefits of the opt-in. The title is currently misleading. Even though it is called base-10 the base value can be overridden to any value. Also I think $clay-font-size-base should have $font-size-base as default. But having two different values for clay base and the default base would break the consistency of the whole UI since clay is very invasive.

[pat270]

I just checked the Bootstrap source for hardcoded rem variables and I only found 2 locations

I'm probably misunderstanding what you mean by hardcoded rem variables. I was assuming any variable with the value {number}rem. A quick search of https://github.com/twbs/bootstrap/blob/8a628b943cf31ca0a002c08af661a95772480225/scss/_variables.scss shows me 94 places.

I have been using Bootstrap 4 based themes with 10px base font size without any problems for years now.

You sure about this? https://github.com/twbs/bootstrap/releases/tag/v4.0.0. Bootstrap 3 was base 10px. Bootstrap 4 is browser default which is generally 16px.

Current themes should not break if $font-size-base is set to 1rem.

By current themes, I'm referring to brand new themes created for 7.1 and not themes from 6.2 or 7.0. We can't be 100% sure of not breaking new themes created for 7.1 since anything could have been done with the variables at this point.

6.2 (Bootstrap 2) and 7.0 (Bootstrap 3) themes are expected to break since Bootstrap 4 is a complete rewrite. We're just trying to minimize the breakages from Bootstrap 3 to Bootstrap 4 which is what the opt-in theme is trying to accomplish.

I don't really know what to say about the hardcoded widgets that cannot be overriden. I assume they are in the same Bootstrap 4 namespace as the rest of the site.

I was referring to Liferay Portal specific CSS (not in the Clay repo) that uses a mix of px and rem values. Applying base 10px will change the sizing of Liferay Portal specific components that use hardcoded rem values that were based on 16px. This is more of an issue for someone that decides to base their 7.1 theme on 10px values.

I am not convinced about the benefits of the opt-in.

The benefit of opt-in essentially allows us to keep the code base the same so we don't break new 7.1 themes. The changes that you are suggesting are basically opt-in since you would need to change the documents font size to 10px and $font-size-base: 1.6rem. My suggestion in the previous comment makes the changes in a separate file so we don't run into unintended consequences for new 7.1 themes.

The title is currently misleading.

I named it quickly to help illustrate my thought process.

[jbalsas]

Hey @pat270, is this long standing PR somewhat related? -> RFS (Responsive font size) implementation

Could we build on top of it?

[pat270]

@jbalsas It looks like it would work. We would have to update our source to use the font-size mixin, but I can't say for sure. I'll grab that pull and test it out.

[pat270]

@jbalsas the RFS feature wasn't what I expected. It scales font-sizes based on viewport width at a specific max-width breakpoint. It doesn't provide any of the things we need for supporting base 10 font size. We can't apply this to things like padding, margins, height, width, etc. It also gives us odd font sizes like 15.2173 which wouldn't fly with the Lexicon team.