incuna / incuna-sass

Incuna's Sass Library
MIT License
2 stars 2 forks source link

amend em-scale to use dynamic list of font sizes and breakpoints #85

Closed hippogriffic closed 7 years ago

hippogriffic commented 7 years ago

tested this locally, seems to work quite nicely, bit more flexible than it was before

@incuna/frontend pls

henrahmagix commented 7 years ago

I think the data structure here is perhaps confusing, considering the comments regarding variable naming for the loop. Maybe we can come up with a nomenclature that aids that understanding? "List of lists" or something. Maybe inner-list would be useful as the @each variable?

henrahmagix commented 7 years ago

Just found out that we can use a "rest parameter" to simplify the usage of the mixin, though the loop definition stays the same:

$sizes-for-breakpoints...
// instead of
$sizes-for-breakpoints: ()
// used as
@include em-scale(12px, ($px768, 16px), ($px1024, 20px), ($px1200, 24px))
// instead of
@include em-scale(12px, (($px768, 16px), ($px1024, 20px), ($px1200, 24px)))

SO answer http://stackoverflow.com/a/15309942/3150057 Docs http://sass-lang.com/documentation/file.SASS_REFERENCE.html#variable_arguments

Taking the examples from #84, we get:

@mixin em-scale($font-size-base, $sizes-for-breakpoints...)
    font:
        size: $font-size-base
    @each $list in $sizes-for-breakpoints
        $breakpoint: nth($list, 1)
        $font-size: nth($list, 2)
        @include media($breakpoint)
            font:
                size: $font-size

$px768: 'screen and (min-width: 768px)'
$px1024: 'screen and (min-width: 1024px)'
$px1200: 'screen and (min-width: 1200px)'

.thing
    @include em-scale(12px, ($px768, 16px), ($px1024, 20px), ($px1200, 24px))

.no-args
    @include em-scale(12px)
.thing {
  font-size: 12px; }
  @media screen and (min-width: 768px) {
    .thing {
      font-size: 16px; } }
  @media screen and (min-width: 1024px) {
    .thing {
      font-size: 20px; } }
  @media screen and (min-width: 1200px) {
    .thing {
      font-size: 24px; } }

.no-args {
  font-size: 12px; }
hippogriffic commented 7 years ago

I have devised a solution please hold the line

hippogriffic commented 7 years ago

@incuna/frontend thoughts now?

LibbyChapman commented 7 years ago

I like it 👍 :star: :star: :star: ✨ 🍰

pandalion commented 7 years ago

I think this can be merged and the warning could be done as an enhancement later

henrahmagix commented 7 years ago

👍