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.

Sign up for GitHub

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

Jump to bottom

LPS-136427 Move Some Custom Properties to Clay CSS #104983

Closed
wants to merge 5 commits into from
Closed

LPS-136427 Move Some Custom Properties to Clay CSS #104983

wants to merge 5 commits into from

Conversation

liferay-continuous-integration
Copy link
Collaborator

Forwarded from: liferay-frontend#1277 (Took 1 ci:forward attempt in 3 minutes)
Console

@pat270
@liferay-frontend

Original pull request comment:
https://issues.liferay.com/browse/LPS-136427

Hey @marcoscv-work @edalgrin @nhpatt @dsanz @matuzalemsteles @drakonux ,

I was able to move a lot of the custom properties to Clay CSS. Some places that are giving me trouble is:

$theme-colors: () !default;
$theme-colors: map-deep-merge(
	(
		primary: $primary,
		secondary: $secondary,
		success: $success,
		info: $info,
		warning: $warning,
		danger: $danger,
		light: $light,
		dark: $dark,
	),
	$theme-colors
);

$container-max-widths: (
	sm: 540px,
	md: 720px,
	lg: 960px,
	xl: 1248px,
) !default;

It's due to Bootstrap functions and mixins. I need to update those, but didn't have enough time today to figure out a work around. Anyway take a look and tell me what you think.

Edit: I've cleaned the custom properties implementation up a lot. I believe we can get rid of the custom_properties directory and bake it directly into Classic Theme. Some things that need to be updated besides the two points above:

  • When you change a color in Stylebook, it adds it to #wrapper:
<style>
  :root {
    --primary: #0b5fff;
  }
  #wrapper {
    --btn-primary-background-color: #0b5fff;
    --font-size-base: 0.875rem;
    --primary: #8e00eb;
  }
</style>

can we change this to replace the custom property inside :root? We don't need to try and scope CSS this way anymore with Cadmin.

  • All property values using a custom property should have a default value. This allows setting the Stylebook value to initial and resetting the property back to its default.
  • We probably don't need to style #wrapper anymore and just use body. I can update Clay CSS so we can customize font-size in the body tag better.
  • Why is --hr-border-color called Separation Border Color in Stylebook? It's so hard to make the connection between that label and the variable name.

Here is some markup you can toss into a fragment for testing:

<h1>h1 Article Heading <small>Sub text</small></h1>
<h2>h2 Article Heading <small>Sub text</small></h2>
<h3>h3 Article Heading <small>Sub text</small></h3>
<h4>h4 Article Heading <small>Sub text</small></h4>
<h5>h5 Article Heading <small>Sub text</small></h5>
<h6>h6 Article Heading <small>Sub text</small></h6>

<h3>Buttons</h3>
<button class="btn btn-primary" type="button">Primary</button>
<button class="btn btn-secondary" type="button">Secondary</button>
<button class="btn btn-link" type="button">Link</button>

<h3>Buttons Focus</h3>
<button class="btn btn-primary focus" type="button">Primary</button>
<button class="btn btn-secondary focus" type="button">Secondary</button>
<button class="btn btn-link focus" type="button">Link</button>

<h3>Buttons Disabled</h3>
<button class="btn btn-primary" disabled type="button">Primary</button>
<button class="btn btn-secondary" disabled type="button">Secondary</button>
<button class="btn btn-link" disabled type="button">Link</button>

<h3>Spacer 0</h3>
<div class="pt-0 bg-dark"></div>
<h3>Spacer 1</h3>
<div class="pt-1 bg-dark"></div>
<h3>Spacer 2</h3>
<div class="pt-2 bg-dark"></div>
<h3>Spacer 3</h3>
<div class="pt-3 bg-dark"></div>
<h3>Spacer 4</h3>
<div class="pt-4 bg-dark"></div>
<h3>Spacer 5</h3>
<div class="pt-5 bg-dark"></div>
<h3>Spacer 6</h3>
<div class="pt-6 bg-dark"></div>
<h3>Spacer 7</h3>
<div class="pt-7 bg-dark"></div>
<h3>Spacer 8</h3>
<div class="pt-8 bg-dark"></div>
<h3>Spacer 9</h3>
<div class="pt-9 bg-dark"></div>
<h3>Spacer 10</h3>
<div class="pt-10 bg-dark"></div>

✔️ ci:test:stable - 9 out of 9 jobs passed

✔️ ci:test:relevant - 22 out of 22 jobs passed in 1 hour 24 minutes

Click here for more details.

Base Branch:

Branch Name: master
Branch GIT ID: 7cb912889719a27e3dff73a0501155c0f36610f9

Upstream Comparison:

Branch GIT ID: 7cb912889719a27e3dff73a0501155c0f36610f9
Jenkins Build URL: Acceptance Upstream DXP (master) #2168

ci:test:stable - 9 out of 9 jobs PASSED
9 Successful Jobs:
ci:test:relevant - 22 out of 22 jobs PASSED
22 Successful Jobs:
For more details click here.
Test bundle downloads:

✔️ ci:test:sf - 1 out of 1 jobs passed in 2 minutes

Click here for more details.

Base Branch:

Branch Name: master
Branch GIT ID: 7cb912889719a27e3dff73a0501155c0f36610f9

Sender Branch:

Branch Name: custom-properties
Branch GIT ID: d363d9a9b65885a7b071e9f47312b7935c7050c6

1 out of 1jobs PASSED
1 Successful Jobs:
For more details click here.

@pat270
LPS-136427 Leverage Clay CSS for some custom properties
15b151c
@pat270
LPS-136427 Classic Theme custom_properties removes duplicate CSS vari…
3663e57 
…ables definitions. Tokens inside frontend-token-definition.json are automatically generated into CSS variables inside a style tag in the head.
@pat270
LPS-136427 Classic Theme custom_properties adds gray-dark, lighter, f…
13bbc79 
…ont-weight-semi-bold to token definitions, so we don't need to rewrite the CSS variables inside the theme.
@pat270
LPS-136427 Classic Theme custom_properties removes custom properties …
b2358ab 
…--transparent, --line-height-base, --line-height-sm. These are not defined in the Style Book. Line Height is also insanely difficult to customize through one variable in a giant application like DXP.
@pat270
LPS-136427 Classic Theme custom_properties clean up CSS and leverage …
d363d9a 
…existing Sass variables

LPS-136427 Classic Theme SF _clay_variables.scss
Closed
@liferay-continuous-integration
Copy link
Collaborator Author

Please only forward critical changes to Brian Chan during stabilization. Nonurgent changes should wait until the ongoing 7.4 DXP EP3 & CE GA3 release has been completed. For more details on the release timeline and status, see product-delivery.

@liferay-continuous-integration
Copy link
Collaborator Author

To conserve resources, the PR Tester does not automatically run for forwarded pull requests.

@brianchandotcom
Copy link
Owner

@pat270 rebase error, please resend? Thx!

@liferay-continuous-integration liferay-continuous-integration deleted the ci-forward-custom-properties-pr-1277-sender-pat270-ts-1627736900009 branch August 8, 2021 10:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
ci:test:relevant - success ci:test:sf - success ci:test:stable - success
Projects
None yet
Milestone
No milestone
3 participants
@liferay-continuous-integration @brianchandotcom @pat270