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

BoxShadow global variables #70

Merged
merged 3 commits into from Mar 23, 2018

Conversation

srambach
Copy link
Member

Adds global variables for BoxShadow and removes the active state shadow on buttons.


$pf-global--sm--BoxShadow: 0 pf-prem(1) pf-prem(2) 0 rgba($pf-color-black-1000, .2) !default;
Copy link
Member

Choose a reason for hiding this comment

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

We don't have a pf-prem function anymore. now it's 2 functions:

  • pf-size-prem
  • pf-font-prem

Depending if you want to set up a size for font or space. In this case I'd use the one for size.

@andresgalante
Copy link
Member

Remember that we need CSS variables. Please create css variables with the same name as the sass variables.

$pf-global--sm-right--BoxShadow: pf-prem(4) 0 pf-prem(10) pf-prem(-4) rgba($pf-color-black-1000, .12) !default;
$pf-global--sm-left--BoxShadow: pf-prem(-4) 0 pf-prem(10) pf-prem(-4) rgba($pf-color-black-1000, .12) !default;
$pf-global--sm-bottom--BoxShadow: 0 pf-prem(4) 0 pf-prem(10) pf-prem(-4) rgba($pf-color-black-1000, .12) !default;
$pf-global--sm-top--BoxShadow: 0 pf-prem(-4) 0 pf-prem(10) pf-prem(-4) rgba($pf-color-black-1000, .12) !default;
Copy link
Member

Choose a reason for hiding this comment

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

these variable names are not consitent, sometimes they have a - others a --.

I was thinking that maybe to make them more redable we can do:
pf-global--concept--modifier--state--PropiertyCamelCase

concept: shadow
modifier: sm, lg, md, sm-left, sm-right, etc

So we end up with:
pf-global--shadow--md-left--BoxShadow

I don't know if it makes much sense. But for the meetup it's ok and we can review naming after we test them.

Copy link
Member Author

Choose a reason for hiding this comment

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

I was thinking they should have --shadow-- in them also but what was there didn't so I didn't :P <3 I added it and fixed the --

@andresgalante andresgalante merged commit 2c8d62f into patternfly:master Mar 23, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants