Skip to content

Refactor all variables as defaults#1536

Merged
juliasolorzano merged 6 commits into
rename-scssfrom
refactor-variables
Oct 27, 2016
Merged

Refactor all variables as defaults#1536
juliasolorzano merged 6 commits into
rename-scssfrom
refactor-variables

Conversation

@shawnbot

Copy link
Copy Markdown
Contributor

⚠️ As configured, this will merge into #1530. If we intend to follow through with this, we should merge that PR first, then set the base branch for this one to staging and rebase. ⚠️

This is a proposed fix for #1535 and #1532. It deprecates core/defaults and core/grid-settings in favor of core/variables with !default variable declarations that allow any variable to be overridden if it's set before being imported.

I'm documenting some of my findings from searching through github for usage examples in #1535, but in the meantime I would love a gut check.

@shawnbot

Copy link
Copy Markdown
Contributor Author

This is definitely a breaking change. It seems safe to deprecate core/defaults in favor of core/variables because very few users seem to be using both of them (and which is a noop anyway), and the way that I've changed things in core/grid-settings shouldn't impact anyone because I haven't seen any instances of users modifying the grid (though I'm sure they're out there).

@juliasolorzano juliasolorzano left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🚀 going to go ahead and merge into the rename-scss branch to keep moving things forward.

$font-bold: 700;
// These variables drive almost every aspect of the U.S. Web Design
// system. Every variable is declared as !default so that it can be
// overridden by a previous import.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I've got an example of this in the new README.md changes here. Once this gets merged in, I'll update here to point to that example. ^5

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 this pull request may close these issues.

2 participants