Introduce logical property handling (sass) #34
Conversation
| @@ -1,9 +1,14 @@ | |||
| @use "font"; | |||
| @use "vendor/sass-rem/rem" as sass-rem with ( | |||
| $rem-baseline: font.$size, | |||
| $rem-fallback: false, | |||
There was a problem hiding this comment.
Now the default, so don't need to provide it as an argument.
src/shared-styles/_rem.scss
Outdated
| ); | ||
|
|
||
| @forward "vendor/sass-rem/rem"; |
There was a problem hiding this comment.
Left over code, not needed now we're wrapping what we use from sass-rem. Will remove.
|
|
||
| @if type-of($arguments) == "list" and length($arguments) > 1 { | ||
|
|
||
| @include validation.expect-at-most($arguments, 2, "More than two arguments supplied with 'inline' dimension") { |
There was a problem hiding this comment.
@include expect.at-most( reads quite nicely. 🤔
There was a problem hiding this comment.
It reads well, but am slightly wary of introducing that as there's already an expect mixin provided by the test framework. Similar argument against assert.at-most which I also considered. Do you think validate.expect-at-most improves on what's currently there?
| @@ -1,17 +1,17 @@ | |||
| @use "test" as *; | |||
| @use "../../src/shared-styles/validation" with ( $is-test: true); | |||
| @use "../../src/shared-styles/logical-properties"; | |||
| @use "../../src/shared-styles/logical"; | |||
There was a problem hiding this comment.
logical seems a reasonable filename / namespace, from https://caniuse.com/#search=logical it doesn't look like it would cause a conceptual clash with anything else. Allows for use as logical.property which reads well.
src/shared-styles/_validation.scss
Outdated
| @@ -0,0 +1,33 @@ | |||
| $is-test: false !default; | |||
There was a problem hiding this comment.
Should maybe call this capture-errors, or, maybe better: throw-errors.
test/sass/validation.spec.scss
Outdated
|
|
||
| } | ||
|
|
||
|
|
test/sass/validation.spec.scss
Outdated
| @include output { | ||
| @include validation.expect-at-most((1, 2, 3), 2) { | ||
| --dev: "null"; | ||
| } | ||
| } | ||
|
|
||
| @include expect { | ||
| _error: "Expected at most 2 values"; | ||
| } | ||
| } | ||
|
|
||
| } | ||
|
|
||
| @include it("does not error when passed no more than the specficied threshold number of values") { | ||
|
|
||
| @include assert() { | ||
|
|
||
| @include output { | ||
| @include validation.expect-at-most((1, 2, 3), 3) { | ||
| --dev: "null"; | ||
| } | ||
| } | ||
|
|
||
| @include expect { | ||
| --dev: "null"; | ||
| } | ||
| } |
There was a problem hiding this comment.
Some indentation problems here.
src/shared-styles/_validation.scss
Outdated
| @function error($message, $capture: $is-test) { | ||
| @if $capture { | ||
| @return $message; | ||
| } | ||
|
|
||
| @error $message; | ||
| } | ||
|
|
||
| @mixin error($message) { | ||
| _error: error($message); | ||
| } |
There was a problem hiding this comment.
Maybe these should be their own module?
There was a problem hiding this comment.
Tried that but I couldn't work out how to pass in $is-test with with, as error is then @used by validation which needs to provide a different $is-test value depending on the calling context. Might just be a sass modules learning point, but going to leave it for now until a clear pattern emerges.
There was a problem hiding this comment.
UPDATE: Got it working in #35.
No description provided.