Skip to content
This repository was archived by the owner on Nov 21, 2019. It is now read-only.

[WIP] Use config module#42

Open
davidcmoulton wants to merge 43 commits intolibero:masterfrom
davidcmoulton:use-config-module
Open

[WIP] Use config module#42
davidcmoulton wants to merge 43 commits intolibero:masterfrom
davidcmoulton:use-config-module

Conversation

@davidcmoulton
Copy link
Contributor

@davidcmoulton davidcmoulton commented Jan 16, 2019

  • Replaces built-in config manager with the dedicated npm module.

  • Requires simultaneous updates to Sass to use values derived from Sass maps rather than now non-existent variables. This approach facilitates increased mixin use, and so increases the innate testability of the Sass codebase. N.B. This step is required to ensure the Travis build will pass

@davidcmoulton
Copy link
Contributor Author

At this stage, the mixins, functions and maps are internally consistent, and obsolete variables have been replaced with map handling.

What remains is to update the patterns' sass, and do a sweep of the mixins to add tests where possible.

@davidcmoulton davidcmoulton requested a review from a team as a code owner March 20, 2019 15:04
$passed: type_of($candidate) == "number";
$passed: $passed and type_of($candidate / 1px);
$passed: $passed and unitless($candidate / 1px);
@return $passed;
Copy link
Contributor

Choose a reason for hiding this comment

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

return type_of($candidate) == "number" and unit($candidate) == "px";?

.stylelintrc \
gulpfile.babel.js \
./
COPY libero-config/ libero-config/
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the actual config should remain here...

"engines": {
"node": ">=10.12"
},
"dependencies": {
Copy link
Contributor

Choose a reason for hiding this comment

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

Should be in devDependencies.


h1 {
@include h1-typography();
@include heading-level-typography(1, 48px);
Copy link
Contributor

Choose a reason for hiding this comment

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

Magic number, at least shouldn't be defined here...

@return map_get($map, $key);
}

@return "[Not found]";
Copy link
Contributor

Choose a reason for hiding this comment

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

Should be an error?


@include contains {
font-size: 10px;
font-size: var(--font-size-h1);
Copy link
Contributor

Choose a reason for hiding this comment

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

Are these meant?

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants