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

Drop color(), theme-color() & gray() functions #29083

Merged
merged 3 commits into from
Jul 25, 2019

Conversation

MartijnCuppens
Copy link
Member

@MartijnCuppens MartijnCuppens commented Jul 20, 2019

Drop color(), theme-color() & gray() functions in favor of variables. The functions just called a map-get() of a map where just the variables were defined.

Also the theme-color-level() now accepts any color you want instead of only $theme-colors colors. The first value now is a variable instead of the $theme-colors key.

TODO:

  • Document breaking change in migration guide
  • Rename theme-color-level() to color-level()

https://deploy-preview-29083--twbs-bootstrap.netlify.com/docs/4.3/getting-started/theming/

@MartijnCuppens MartijnCuppens marked this pull request as ready for review July 20, 2019 09:48
@MartijnCuppens MartijnCuppens requested a review from a team as a code owner July 20, 2019 09:48
@mdo
Copy link
Member

mdo commented Jul 21, 2019

Also the theme-color-level() now accepts any color you want instead of only $theme-colors colors.

Should this function be renamed to color-level() then?

@MartijnCuppens
Copy link
Member Author

Should this function be renamed to color-level() then?

Definitely

@ysds
Copy link
Member

ysds commented Jul 22, 2019

@mdo Do you have any plans for #28268?

@MartijnCuppens MartijnCuppens force-pushed the master-mc-cleanup-functions branch 3 times, most recently from 2d4b302 to 47ee60d Compare July 22, 2019 19:46
@mdo
Copy link
Member

mdo commented Jul 23, 2019

@ysds Yes, I would like to see that come back. The idea isn't to provide those classes to everyone though—just the variables and functions. The classes are purely there for docs presentation.

@ysds
Copy link
Member

ysds commented Jul 24, 2019

@mdo OK, thanks 👍

@MartijnCuppens Can you confirm the changes in the migration.md? Other than that, LGTM!

@MartijnCuppens
Copy link
Member Author

Yup, it looks ok

@MartijnCuppens
Copy link
Member Author

I just spotted some unwanted changes in migration.md, gonna fix that first

MartijnCuppens and others added 2 commits July 24, 2019 18:47
…ables

Also the `theme-color-level()` now accepts any color you want instead of only `$theme-color` colors.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants