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

Remove deprecation warning from `reset-display` #456

Merged
merged 1 commit into from Jul 8, 2016

Conversation

Projects
None yet
5 participants
@whmii
Member

whmii commented Jul 7, 2016

@whmii whmii changed the title from Remove depreciation warning from `reset-display` to Remove deprecation warning from `reset-display` Jul 7, 2016

@tysongach

This comment has been minimized.

Member

tysongach commented Jul 7, 2016

Shouldn’t we just be able to remove lines 3840 in _row.scss?

I’m a bit confused, but I don’t think that reset-display is attributed to the issue.

@tysongach

This comment has been minimized.

Member

tysongach commented Jul 7, 2016

The reset-display mixin is not used in any other mixin in Neat except for reset-all. So I don’t think it has no affect on the row mixin or its deprecated arguments.

@whmii whmii force-pushed the whmii-warnings branch from 77de023 to a51c787 Jul 8, 2016

@whmii whmii force-pushed the whmii-warnings branch from a51c787 to 80a98ee Jul 8, 2016

@whmii whmii merged commit 80a98ee into master Jul 8, 2016

1 of 2 checks passed

ci/circleci CircleCI is running your tests
Details
hound No violations found. Woof!

@whmii whmii deleted the whmii-warnings branch Jul 8, 2016

@tysongach

This comment has been minimized.

Member

tysongach commented Jul 8, 2016

@whmii Any word on my comments above? Am I mistaken on how that works?

@Sxderp

This comment has been minimized.

Sxderp commented Jul 10, 2016

@tysongach The problem is the global variable $container-is-display-table.

The row() mixin changes this variable (line 39). The reset-display() also changes this variable (line 13).

When using row, you have to manually call reset-display() in order to reset the global variable. Before the most recent commits, they were trying to push the display-context() mixin which would have done this for you. The problem they had was trying to support the 'deprecated' method as well (keeping the globals in row and not using is-display-table()).

@dcalhoun

This comment has been minimized.

dcalhoun commented Feb 28, 2017

@whmii I am a little confused about what is the correct usage of display.

Example 1

@include row(table)

Example 2

@include display-context(table) {
  @include row();
}

Example 1 throws a deprecation warning. Example 2 appears to have no impact on the display for the row. What is the correct usage of display in v1.8.0?

@kgcreative

This comment has been minimized.

Contributor

kgcreative commented Feb 28, 2017

Deprecation warnings don't affect 1.x - they are there to let you know which features will not be present in the next major release

@dcalhoun

This comment has been minimized.

dcalhoun commented Feb 28, 2017

@kgcreative Understood. From my experience, library deprecations warnings can be resolved prior to upgrading. The idea being that if you "take care of" all the warnings, upgrading to the major release is a straight-forward process. The deprecation warnings are a "checklist" to get ready for the upgrade.

It would appear that it is impossible to resolve Neat's "deprecation warnings" without upgrading to "the next major release", which sort of defeats the purpose of including them in my compilation logs for 1.x.x IMO.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment