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

Add CSS color variables to Card component #35097

Closed
wants to merge 4 commits into from

Conversation

tagliala
Copy link
Contributor

@tagliala tagliala commented Oct 3, 2021

Hi, this is an attempt to provide variables for the Card component, but I have a couple of questions

  • $card-color and $card-cap-color are defined as null. This has the effect of rules being generated with SCSS, but it will generate empty css variables.
    • Is it fine to have empty CSS variables?
    • Should the default value change to inherit?
    • if the default value change from null to inherit, should it be considered a breaking change?
  1. Should the conditional rules in @if $nav-tabs-link-active-bg != $card-bg moved out from if?

@mdo
Copy link
Member

mdo commented Oct 13, 2021

This is off to a great start! For null variables, I'd use an inline if statement to have an empty value if null. It's fine to have empty ones at this time IMO. Unclear about that second question... I feel like we shouldn't have conditionals like that.

@tagliala
Copy link
Contributor Author

Hi!

Apologies for the silence

For null variables, I'd use an inline if statement to have an empty value if null

Could you please clarify this?

@mdo
Copy link
Member

mdo commented Nov 2, 2021

@tagliala I meant something like this:

cursor: if($enable-button-pointers, pointer, null);

That said, it might be fine to have some unused variables lying around for now provided we have a documented approach for them. Unused or null variables in Sass exist in case someone wants to customize. I think given our hybrid CSS vars / Sass solution in v5, we can keep this going, I'm just not locked in on any one direction for it.

@mdo mdo mentioned this pull request Nov 2, 2021
41 tasks
@tagliala
Copy link
Contributor Author

Hello, I've rebased this PR on master and I've replaced all card scss variables into css variables

About the definitions, I've used the same order of scss variables, but I know that the css is different

The conversion is based on an automatic find and replace to prevent typos

Feel free to take over this PR

@mdo mdo mentioned this pull request Mar 24, 2022
@mdo
Copy link
Member

mdo commented Mar 24, 2022

Closed for #36059.

@mdo mdo closed this Mar 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants