Page MenuHomePhabricator

Create visalization for line height
Closed, ResolvedPublic

Description

Get inspiration from the lightning design system linked in the parent task.

Event Timeline

Leaving here a couple of improvements:

  1. Make the font size of the value column 14px.
  2. It may make sense to use typeset here too (like in the rest of the font pages) rather than the blue background. If it does, then I would apply the following sizes to each specimen (none: 16px, 1.25: 18px, 1.5: 16px, 1.6: 14px)

Leaving here a couple of improvements:

  1. Make the font size of the value column 14px.
  2. It may make sense to use typeset here too (like in the rest of the font pages) rather than the blue background. If it does, then I would apply the following sizes to each specimen (none: 16px, 1.25: 18px, 1.5: 16px, 1.6: 14px)

https://github.com/wmde/wikit/pull/60

Checked! Looks good. Ready to move to done.

@Sarai-WMDE I have a question about

apply the following sizes to each specimen (none: 16px, 1.25: 18px, 1.5: 16px, 1.6: 14px)

We are changing the font size depending on the token value (line-height), thereby changing two variables at once and making it hard to grasp the impact of one or the other. Are we sure this is in the best interest of the user?

Thanks

In a comment on the PR

we can go back to using 16px:

and again during a conversation after the daily, it was decided to use the same font size, 16px, to present all line heights.

In a comment on the PR

we can go back to using 16px:

and again during a conversation after the daily, it was decided to use the same font size, 16px, to present all line heights.

An explanation was provided why there are different font sizes, which answers your initial why @Pablo-WMDE
The comment from the PR with a context

But since said block cannot be used, I'd agree that we can go back to using 16px: although imprecise.

Bending requirements and making the implementation imprecise in favor of code consistency is fundamentally wrong.
Not using Typeset block is not a problem. We have other presenters which do not come from Storybook's blocks as well.

I wrote down what was said during a call of 3 people (= messenger). I don't know why the requirement was changed again despite the explanation (implying it was intentional).

During that conversation it was also mentioned that a more useful way of illustrating the relationship between line height and font size could be to show them in a matrix (i.e. show [the same] set of multiple font sizes for each line height, instead of one arbitrary font size). This idea was not turned into a requirement, however.

But since said block cannot be used, I'd agree that we can go back to using 16px: although imprecise.

Bending requirements and making the implementation imprecise in favor of code consistency is fundamentally wrong.

I largely agree. However, it was a designer, not a developer who wrote the original quote - and they make the requirements.

@Sarai-WMDE

I tried to incorporate the new decision into https://github.com/wmde/wikit/pull/60 - please verify visually.

Also created https://github.com/wmde/wikit/pull/68 which shows the idea as discussed in the meeting, demoing each line height at 3 font sizes (the ones mentioned earlier). Don't know if anyone needs that but had some time during a meeting which I had no business being in. Take a look at the storybook, please, and decide if this solves the problem you initially wanted to tackle.

@Sarai-WMDE

I tried to incorporate the new decision into https://github.com/wmde/wikit/pull/60 - please verify visually.

Also created https://github.com/wmde/wikit/pull/68 which shows the idea as discussed in the meeting, demoing each line height at 3 font sizes (the ones mentioned earlier). Don't know if anyone needs that but had some time during a meeting which I had no business being in. Take a look at the storybook, please, and decide if this solves the problem you initially wanted to tackle.

In light of https://github.com/wmde/wikit/pull/60 being merged and https://github.com/wmde/wikit/pull/68 receiving a comment by UX not approving of the proposal, I consider this task done and will move it to the appropriate column.