Line height in Typeset component is broken

[coderitual]

Describe the bug Line height in Typeset component is broken. Typeset component should allow to provide lineHeights prop which will be corresponding to fontSizes prop. That way we can create proper presentation of font (we always need to define lineHeight to get the best look from the font for a given fontSizes)

Currently it is set based on size, but there is a bug (no px suffix so the unit is relative by default - for fontSize, number is px by default).

Previously its value was fixed to 1.

https://github.com/storybookjs/storybook/commit/926b68f1e538c0058a889ad2454436821297af36

Expected behavior Exposed lineHeights prop in Typeset component

Screenshots

Code snippets https://github.com/storybookjs/storybook/commit/926b68f1e538c0058a889ad2454436821297af36

[shilman]

@jpzwarte looks like this was introduced in #12134. mind taking a look?

[jpzwarte]

@shilman @coderitual I cannot reproduce this. My code:

<Typeset fontFamily="open-sans" fontSizes={['12px', '14px', '16px']}></Typeset>

Which results in:

@coderitual Are you specifying font-size without a unit? Afaict that is not valid (unless it is a percentage)? https://developer.mozilla.org/en-US/docs/Web/CSS/font-size

[coderitual]

@jpzwarte in css font-size without unit is invalid - agree. In react you don't need it as px suffix is added automatically to inline properties which support that unit -> https://reactjs.org/docs/dom-elements.html#style

I also think that setting line-height property to the same value as font size is not correct from design point of view. It would be better to leave it to default value which is normal (and differs depending on font-family).

It would be awesome to have lineHeights prop also exposed as in design systems world, font sizing token is always defined as a pair of size and line-height. That way desingers provide the best looking version of font for given font-size in multiline blocks of text (it's called vertical rythm - https://iamsteve.me/blog/entry/a-guide-to-vertical-rhythm).

[jpzwarte]

@coderitual Setting line-height to the same value as font-size is meant to fix this bug: https://github.com/storybookjs/storybook/pull/12134

A "workaround" for your problem would be to specify a valid font-size (from a CSS perspective).

Feel free to submit a PR that allows you to customize the line-height. The storybook devs are pretty open to such PRs in my experience :)

[coderitual]

thanks @jpzwarte :)

[stale[bot]]

Hi everyone! Seems like there hasn't been much going on in this issue lately. If there are still questions, comments, or bugs, please feel free to continue the discussion. Unfortunately, we don't have time to get to every issue. We are always open to contributions so please send us a pull request if you would like to help. Inactive issues will be closed after 30 days. Thanks!

[Ryuno-Ki]

Can we reactivate this?

[casey-lentz]

Looking forward to this update.

[marymettille]

Is this still something coming down the pipeline? VERY interested in it!