Financial-Times / o-typography

Typography and vertical rhythm styles for FT branding
http://registry.origami.ft.com/components/o-typography
10 stars 2 forks source link

Remove oTypographySize. #214

Closed notlee closed 5 years ago

notlee commented 5 years ago

We introduced multiple font scales for specialist titles earlier in the year, so the result of oTypographySize depends on the font.

Instead of passing a font type to oTypographySize use oTypographySans, oTypographySerif, or oTypographyDisplay with arguments instead. This also includes progressive font loading styles for the updated size, where oTypographySize did not.

origamiserviceuser commented 5 years ago

o-typography bundle size difference from 5.12.0 to 18a96f7db11344f7dd62233051b3fb71222cd137 css, master: 13.82kb decrease (-1.09kb/gzip) css, internal: 9.92kb decrease (-0.53kb/gzip) css, whitelabel: 1.04kb decrease (-0.05kb/gzip) An insignificant difference was also found for: js

notlee commented 5 years ago

This also includes progressive font loading styles for the updated size, where oTypographySize did not.

I'm unsure what I think about this. On the one hand it could make for a better looking more reliable UI when FT fonts haven't loaded yet. On the other hand it will bloat the user's CSS bundle.

Wish we could rely on font-display!

Example progressive font loading font-size adjustment:

.my-type {
    @include oTypographySans($scale: 1, $opts: ('font-family': false));
}
.my-type {
    font-size: 18px;
    line-height: 1.6;

    .o-typography--loading-sans .my-type {
        font-size: 15.66px;
    }
}
origamiserviceuser commented 5 years ago

o-typography bundle size difference from 5.12.0 to 78f4c75094be98450b3bf6757bb6f9cb3ffbfa0d css, master: 13.82kb decrease (-1.09kb/gzip) css, internal: 9.92kb decrease (-0.53kb/gzip) css, whitelabel: 1.04kb decrease (-0.05kb/gzip) An insignificant difference was also found for: js

notlee commented 5 years ago

I've discussed some options with @rowanmanning

 /// -  present
@include oTypographySans($scale: 1); // font-family, size, line-height
@include oTypographySize($scale: 1, $font: 'sans'); // size, line-height

 /// -  option 1
 @include oTypographySans($scale: 1); // family, size, line-height
 @include oTypographySans($scale: 1, $opts: ('font-family': false)); // size, line-height

 /// -  option 2 
 @include oTypographySans($scale: 1); // size, line-height
 @include oTypographySans($scale: 1, $include-font-family: true); // family, size, line-height

 /// -  option 3
 @include oTypographySans($scale: 1); // family, size, line-height
 @include oTypographySans($scale: 1, $include-font-family: false); // size, line-height

Option 1 is clunky, it will encourage users to repeat the font family more excessively. Option 2, where the font family is not output by default, is more explicit in what the user wants and it therefore encourages only outputting the font family where needed. The migration will be more difficult and it's less intuitive we feel that a Sans mixin does not output a font family. Option 3, where the font family is included by default but may be excluded more easily, we think is the correct balance.

origamiserviceuser commented 5 years ago

o-typography bundle size difference from 5.12.0 to 5046ac8b278fde198dfb6caebdda8435a2ede81f css, master: 13.82kb decrease (-1.09kb/gzip) css, internal: 9.92kb decrease (-0.53kb/gzip) css, whitelabel: 1.04kb decrease (-0.05kb/gzip) An insignificant difference was also found for: js

notlee commented 5 years ago

Oops, sorry, I got a little merge happy and just merged an approved PR which was based on this PR

origamiserviceuser commented 5 years ago

o-typography bundle size difference from 5.12.0 to dde1d84e1a8afc9a87d16da832ffd1baa376fe10 css, master: 14.3kb decrease (-1.16kb/gzip) css, internal: 10.85kb decrease (-0.60kb/gzip) css, whitelabel: 1.4kb decrease (-0.08kb/gzip) An insignificant difference was also found for: js

origamiserviceuser commented 5 years ago

o-typography bundle size difference from 5.12.0 to 8847dcfcaf68b6deb455c1237613b3821e9e7d69 css, master: 14.3kb decrease (-1.16kb/gzip) css, internal: 10.85kb decrease (-0.60kb/gzip) css, whitelabel: 1.4kb decrease (-0.08kb/gzip) An insignificant difference was also found for: js