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

Improve css-variables support in sass files #52

Closed
bugy opened this issue Nov 23, 2020 · 9 comments
Closed

Improve css-variables support in sass files #52

bugy opened this issue Nov 23, 2020 · 9 comments

Comments

@bugy
Copy link

bugy commented Nov 23, 2020

Hi, I'd like to propose a small sass code change (which I could implement myself if you agree), please find the rationale and suggestion below.

Use case description

I'm trying to use css variables for dynamic theme switching. To make it possible, I'd like to override materialize sass variables, e.g.:

$secondary-color: var(--secondary-color);
$secondary-color-lighten-4: var(--secondary-color-lighten-4);
$button-background-focus: var(--secondary-color-lighten-1);

These are my custom variables and they are taking precedence over materializecss variables. This works perfectly fine for me.

The problem

However, I cannot use color(var(--secondary-color)), because I'd like --secondary-color to be kept as it is, without preprocessing. I.e. in the output css I'd like to have smth like:

.button {
    background-color: var(--secondary-color);
}

But without color(...), there is a problem with other variables and styles, since color functions are not working:

$primary-color-light: lighten($primary-color, 15%) !default;

.progress {
    background-color: lighten($progress-bar-color, 40%);
}

This lighten function fails, since it expects a color as input
For variables, this is not a problem, since I can easily override them

However, for css styles, I wasn't able to find a way, how to make it compilable.

Suggestion

What do you think about avoiding color functions in style files and to move all of them into variables?
For the example above, with the progress bar, it would look like:

$progress-bar-panel-color: lighten($progress-bar-color, 40%) default!;

.progress {
    background-color: $progress-bar-panel-color;
}

I quickly checked the code, and usually there 1-2 color functions in sass files, so this shouldn't be a big change.

What do you think?

@bugy
Copy link
Author

bugy commented Dec 4, 2020

@DanielRuf would such change make sense?
To be honest I don't know who is responsible now for making decisions on the direction of project development

@DanielRuf
Copy link

@materializecss/members

@Pierstoval
Copy link

I think this might be overkill to introduce new shades for every usage of color functions, but I agree that this might be a good solution.

I feel like this is somehow automatically feasible as long as we keep only 5 lightening and 5 darkening shades for every color.
However, this might bloat the final code a bit more...

@doughballs
Copy link

I’m not sure how helpful I can be with regards to colours as my preference would be to remove them altogether! No project needs access to so many colours. I actually strip out all but the base colours in my projects, and even that is bloated. In truth? We receive briefs that will choose colours outside of the ones provided my materialize, and so for some they will always be redundant.

As for removing colour functions from style, I don’t know about the knock on effects but I think I’m in favour!

@Pierstoval
Copy link

@doughballs I just made a suggestion on #54 about removing colors 😉

@doughballs
Copy link

@doughballs I just made a suggestion on #54 about removing colors 😉

Haha great minds!

@bugy
Copy link
Author

bugy commented Dec 4, 2020

Thanks for the quick response:

I run a regex match for color functions in scss folder, and these are results:

_buttons.scss:60:    background-color: darken($button-raised-background, 10%);
forms/_switches.scss:60:    background-color: transparentize($switch-bg-color, .85);
forms/_switches.scss:73:  background-color: transparentize($switch-bg-color, .85);
forms/_range.scss:98:  box-shadow: 0 0 0 10px rgba($radio-fill-color, .26);
forms/_range.scss:129:  box-shadow: 0 0 0 10px rgba($radio-fill-color, .26);
forms/_range.scss:160:  box-shadow: 0 0 0 10px rgba($radio-fill-color, .26);
_tabs.scss:50:        background-color: transparentize($tabs-underline-color, .8);
_tabs.scss:60:      color: rgba($tabs-text-color, .7);
_tabs.scss:73:      color: rgba($tabs-text-color, .4);
_sidenav.scss:62:    &.btn-large:hover { background-color: lighten($button-raised-background, 5%); }
_timepicker.scss:103:	background-color: transparentize($secondary-color, .75);
_dropdown.scss:71:    background-color: darken($dropdown-hover-bg-color, 8%);
_global.scss:650:    background-color: lighten($progress-bar-color, 40%);

May be as a quick solution (because #54 will take some time, especially due to potential regression issues), we can refactor only these colors?

bugy added a commit to bugy/materialize that referenced this issue Dec 6, 2020
…ariables) + adjusted hover/focus to better reflect latest material design
@bugy
Copy link
Author

bugy commented Dec 7, 2020

Hi @doughballs @Pierstoval I created a PR (#55). If you would have time for review and discussion, it would be awesome

bugy added a commit to bugy/materialize that referenced this issue Dec 7, 2020
Co-authored-by: Alex Rock <pierstoval@gmail.com>
bugy added a commit to bugy/materialize that referenced this issue Dec 8, 2020
bugy added a commit to bugy/materialize that referenced this issue Dec 9, 2020
bugy added a commit to bugy/materialize that referenced this issue Mar 22, 2021
…ariables) + adjusted hover/focus to better reflect latest material design
bugy added a commit to bugy/materialize that referenced this issue Mar 22, 2021
Co-authored-by: Alex Rock <pierstoval@gmail.com>
bugy added a commit to bugy/materialize that referenced this issue Mar 22, 2021
bugy added a commit to bugy/materialize that referenced this issue Mar 22, 2021
bugy added a commit to bugy/materialize that referenced this issue Oct 3, 2021
…ariables) + adjusted hover/focus to better reflect latest material design
bugy added a commit to bugy/materialize that referenced this issue Oct 3, 2021
Co-authored-by: Alex Rock <pierstoval@gmail.com>
bugy added a commit to bugy/materialize that referenced this issue Oct 3, 2021
bugy added a commit to bugy/materialize that referenced this issue Oct 3, 2021
bugy added a commit to bugy/materialize that referenced this issue Oct 22, 2021
…tyles (moved to variables) + adjusted hover/focus to better reflect latest material design
bugy added a commit to bugy/materialize that referenced this issue Oct 22, 2021
bugy added a commit to bugy/materialize that referenced this issue Oct 22, 2021
bugy added a commit to bugy/materialize that referenced this issue Oct 22, 2021
bugy added a commit to bugy/materialize that referenced this issue Oct 22, 2021
bugy added a commit to bugy/materialize that referenced this issue Oct 22, 2021
bugy added a commit to bugy/materialize that referenced this issue Oct 25, 2021
bugy added a commit to bugy/materialize that referenced this issue Oct 25, 2021
@LoganTann
Copy link

fixed in #213

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 a pull request may close this issue.

5 participants