onedesign / one-sass-toolkit

A collection of foundational utilities for Sass.
5 stars 2 forks source link

Feature/type style improvements #10

Closed brianjhanson closed 7 years ago

brianjhanson commented 7 years ago

The type style mixing has been bugging me for awhile, it felt a little too complex and too perscriptive. This makes some updates to it, changing the idea of a "style" from a only a few properties to a collection of arbitrary properties.

This changes the configuration to look more like:

$type-styles: (
  heading: (
    stack: sans-serif,
    smoothing: true,
    sizes: (24, 36),
    properties: (
      line-height: 1.4,
      text-transform: normal,
      letter-spacing: 0
    )
  ),

  body: (
    stack: serif,
    sizes: (16),
    properties: (
      line-height: 1.6,
      text-transform: normal,
      letter-spacing: 0,
    )
  )
) !default;

The big things that change:

  1. You can now pass an arbitrary collection of properties to a style. We don't care what they are or if they're valid, they all just go through.
  2. Sizes is now a list and the dependency on sass-mq has been removed. If a list is more than a single number, we interpolate from the first item to the last item (2 items is the desired length, but we don't validate). The start and end interpolation screen sizes are set by new $fluid-min-screen and $fluid-max-screen variables.
  3. Fluid sizing can be easily overridden, for a style with multiple sizes with a new argument to the type-styles mixin. This eliminates the need for a separate get-type-style mixing.
  4. I also simplified the $font-stacks variable to remove font-weight and font-style properties. Those properties are better set when including the typeface with @font-face.

I think this should be agreed upon before merge, so please if you have any thoughts / ideas / concerns, let's use this PR (or dev retro) to discuss.

cmalven commented 7 years ago

@brianjhanson the change to properties makes perfect sense to me.

The changes to how font-sizing works are so different, and in some ways just as prescriptive but prescriptive for a different way of handling type sizing, that I need some time to think this through. There are definitely instances where I want to use fluid sizing, but those instances are almost always for display type (i.e. the exception rather than the rule).

I also simplified the $font-stacks variable to remove font-weight and font-style properties. Those properties are better set when including the typeface with @font-face.

Will a user absolutely, 100% of the time have the ability to set these in @fontface? Or will it always make sense to? What about Typekit? Cloud Typography? Fonts.com? You never had to include these in the $font-stacks, but it seems like you would sometimes, especially if, for instance, you were using Helvetica as we are in the example. Right?

brianjhanson commented 7 years ago

@cmalven I think if we want it to be the exception it's as easy as setting $fluid: false rather than $fluid: true here. We could also alter sizes to accept either a single value or a list (rather than always a list, but that list is allowed to only have one value).

I think right now, even accepting the font-weight and font-style in the $font-stacks is encouraging a bad practice. I haven't need to set those on the last 3 - 4 projects. We also don't need to worry too much about the cloud services, those should all behave correctly if you set font-weight anywhere (in which case it's part of the style rather than the stack).

We only have to worry about manually importing the fonts, and in that case I think it's always better to set it on the @font-face so the browser knows what weight each file is. I haven't researched it too deeply, but I can dive into it further if it's a concern.

brianjhanson commented 7 years ago

Also, I probably bit off a bit too much in this PR. I will work to separate it out a bit more so we can merge in the properties change, then worry about the other stuff. For now, I have published this under the next tag to test it out.

I'm hoping to tweak it a bit as I work through Belgravia.

cmalven commented 7 years ago

@brianjhanson breaking these up into smaller concerns sounds good.