Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Type sizing #62

Closed
mrmartineau opened this issue Nov 5, 2015 · 7 comments
Closed

Type sizing #62

mrmartineau opened this issue Nov 5, 2015 · 7 comments
Assignees
Milestone

Comments

@mrmartineau
Copy link
Member

I saw on Dropbox's Scooter framework, that they have helper classes for type size. We have kind of have this at the moment with our headings:

// Use the helper classes to emulate styles for another element
// For example: h3.h1 or h3.alpha
h1, .h1, .alpha {
    @include font-size($type-largest);
    margin-bottom: $baseline * 2;
}

h2, .h2, .beta {
    @include font-size($type-large);
}

h3, .h3, .gamma {
    @include font-size($type-mid);
}

h4, .h4, .delta {
    @include font-size($type-base);
}

h5, .h5, .epsilon,
h6, .h6, .zeta {
    @include font-size($type-small);
    margin-bottom: 0;
}

What do you think of removing the helper classes from the above and making more generi classes like below:

/**
 * Type sizes
 */
.type-micro    { @include font-size($type-micro); }
.type-smallest { @include font-size($type-smallest); }
.type-smaller  { @include font-size($type-smaller); }
.type-small    { @include font-size($type-small); }
.type-base     { @include font-size($type-base); }
.type-mid      { @include font-size($type-mid); }
.type-large    { @include font-size($type-large); }
.type-largest  { @include font-size($type-largest); }
.type-jumbo    { @include font-size($type-jumbo); }

We could even have this so it is made perfectly clear what sizes each element uses:

/**
 * Type sizes
 */
.type-micro         { @include font-size($type-micro); }
.type-smallest      { @include font-size($type-smallest); }
.type-smaller       { @include font-size($type-smaller); }
.type-small, h5, h6 { @include font-size($type-small); }
.type-base, p, h4   { @include font-size($type-base); }
.type-mid, h3       { @include font-size($type-mid); }
.type-large, h2     { @include font-size($type-large); }
.type-largest, h1   { @include font-size($type-largest); }
.type-jumbo         { @include font-size($type-jumbo); }

Which would ultimately look something like this:

/**
 * Paragraphs
 */
p {
    font-family: $font-family-base;
    margin-top: 0;
    margin-bottom: $baseline;

    // Measure - ideally about 65 chars per line
    // max-width: $type-base * 30 + px;

    // & small {
    //  color: lighten($color-text, 10%);
    // }
}

/**
 * Headings
 */
h1, h2, h3, h4, h5, h6 {
    margin: 0;
    font-family: $font-family-headings;
    font-weight: $font-weight-headings;
    line-height: $line-height-headings;
    text-rendering: optimizelegibility; // Fix the character spacing for headings
    margin-top: 0;
    margin-bottom: $baseline;

    small {
        font-weight: normal;
    }
}

// Use the helper classes to emulate styles for another element
// For example: h3.h1 or h3.alpha
h1 {
    margin-bottom: $baseline * 2;
}

h5
h6 {
    margin-bottom: 0;
}

// Only give these headings a margin-top if they are after other elements
* + h1, * + .h1, * + .alpha,
* + h2, * + .h2, * + .beta,
* + h3, * + .h3, * + .gamma,
* + h4, * + .h4, * + .delta {
    margin-top: $baseline;
}

small {
    font-size: 80%;
}

/**
 * Type sizes
 */
.type-micro                { @include font-size($type-micro); }
.type-smallest             { @include font-size($type-smallest); }
.type-smaller              { @include font-size($type-smaller); }
.type-small, h5, h6, small { @include font-size($type-small); }
.type-base, p, h4          { @include font-size($type-base); }
.type-mid, h3              { @include font-size($type-mid); }
.type-large, h2            { @include font-size($type-large); }
.type-largest, h1          { @include font-size($type-largest); }
.type-jumbo                { @include font-size($type-jumbo); }

Would love some feedback on this please @munkychop @CiaranPark @ashleynolan @nicbell

@mrmartineau mrmartineau changed the title Minor typography amends Type sizing Nov 5, 2015
@mrmartineau mrmartineau modified the milestones: v5.1.0, v6.0.0 Nov 5, 2015
@mrmartineau mrmartineau self-assigned this Nov 5, 2015
@ashleynolan
Copy link
Contributor

Not sure on this one – but I think it depends on how you think changing them would be useful.

Most of the time, the .h1/.alpha classes should be used for overriding the base header sizes. By making them more generic, it’s encouraging more use of classes on elements outside of this to set font-size.

I’m unsure if this is a good thing or not – personally, I would rather people use the $type- variables in their Sass than to use .type- classes in the HTML. At least that way if you ever change one of them and need to see what it will affect, you don’t have to then search throughout your markup for what components use that type-size – instead it’s all within your CSS.

Not sure if I’m missing something that makes more of a case for the pros in changing them though?

@CiaranPark
Copy link

Agreed, I think we should try to enforce more rules on good typography and sizing. Allowing helper classes like this would perhaps mean lot's of shoe-horning of the design and weird font-sizing? I like it is currently, just need to be more strict with them personally.

@mrmartineau
Copy link
Member Author

you don’t have to then search throughout your markup for what components use that type-size – instead it’s all within your CSS.

@ashleynolan isn't this the case already? These would replace the .h1 and .alpha to something that is more universal/able to be used on smaller sizes.

I like the idea of being able to give an element the font-size of an h1 but not all it's other styling (margins etc). I also think that it more clearly shows users what each major element's font sizes are.

@ashleynolan
Copy link
Contributor

It’s the case for headers, for sure, but it would then become a larger issue with type-specific classes being used potentially for anything. I think having header classes like .h1/.alpha isn’t ideal either, but it’s a common use-case and so solves a fixed problem.

I think it is making the markup more tightly coupled to a styling variable that is prone to change and could potentially cause issues down the line. Changing the size of one of my type variables would require me to hunt down uses throughout the markup and possibly JS (if people added classes/markup in there too). Again, this would happen with the heading helper classes currently but at least by it’s definition it’s implied to be limited to use with headers.

I just think font-size, like colour, is too vulnerable to change from my experience – having helper layout classes is more robust and makes sense – it would be like having a .color-primary class to add to elements. It makes things more fragile.

@mrmartineau
Copy link
Member Author

OK, you make a good case, I do not think this is a good idea, but should we remove the .h1 / .alpha classes then? Devs can add them if they need to..

@ashleynolan
Copy link
Contributor

I don’t mind – I guess it’s weighing up the usefulness versus the fragility of them.

I think they’re pretty useful just for header sizing, but if we think it’d be better to get rid of them – or at least one of them as I don’t think we need both in there – then that could be a good idea.

@mrmartineau
Copy link
Member Author

OK, @CiaranPark, what do you think?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants