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 deprecate() mixin #28092

Merged
merged 6 commits into from
Jan 22, 2019
Merged

Add deprecate() mixin #28092

merged 6 commits into from
Jan 22, 2019

Conversation

MartijnCuppens
Copy link
Member

We're probably going to drop a lot of mixins, and maybe some functions in v5. That's why I would like to introduce a deprecate() mixin. This mixin will allow us to deprecate mixins and functions now. Its goal is that all functions/mixins that will be droped in v5 include this mixin so that it'll be easy to check if people are relying on deprecated stuff in their codebase if they want to upgrade from Bootstrap 4 to 5. (They can update to the latest v4 first where all the deprecate() mixins are included.)

I've put #28072, #28067 and #28066 hold for this (I acctually tought $ignore-warning was a global variable, which wasn't the case). Probably going to submit a lot more deprecation PRs (these won't be blocking the 4.3 release)

// Some mixins and functions will be removed in v5, the `deprecate()` mixin will warn you if you're using one of these.
// These warnings can be suppressed by setting this variable to `false`:

$show-deprecation-messages: true !default;
Copy link
Member

Choose a reason for hiding this comment

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

Should we stick with $enable- here?

Also, this is set to true, but shouldn't it be set fo false to start? This replaces $ignore-warning != true, so it's disabled to start I believe.

Copy link
Member Author

@MartijnCuppens MartijnCuppens Jan 21, 2019

Choose a reason for hiding this comment

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

It doesn't replace $ignore-warning != true. That variable is still used to suppress the warning because .text-hide still uses the mixin:

.text-hide {
@include text-hide($ignore-warning: true);
}

($ignore-warning shouldn't be set here, I've opened #28098 for this) Edit: it's clearer to define the variable, I'll just leave it.

I've changed the variable to an $enable-* variable and set it to false by default. Everything should be clear if you look at the changed code, if not, let me know what need to be clarified more.

Copy link
Member

@mdo mdo left a comment

Choose a reason for hiding this comment

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

Couple copy changes here, but otherwise looking good. Appreciate the mixin approach!

scss/mixins/_deprecate.scss Outdated Show resolved Hide resolved
site/docs/4.2/getting-started/theming.md Outdated Show resolved Hide resolved
mdo and others added 2 commits January 22, 2019 18:11
Co-Authored-By: MartijnCuppens <martijn.cuppens@gmail.com>
Co-Authored-By: MartijnCuppens <martijn.cuppens@gmail.com>
@MartijnCuppens MartijnCuppens merged commit 5c56e9a into v4-dev Jan 22, 2019
@MartijnCuppens MartijnCuppens deleted the v4-dev-mc-deprecate branch January 22, 2019 19:55
@mdo mdo mentioned this pull request Jan 22, 2019
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