Page MenuHomePhabricator

TypeaheadSearch line-height, thumbnail size don't match Figma spec or each other
Closed, ResolvedPublic3 Estimated Story Points

Description

As @AnneT pointed our on Gerrit, a recent change applying line-height tokens in TypeaheadSearch changed the design and caused a mismatch. But even before that, we already weren't following the design correctly. We should fix both of those things.

Currently (after the recent line-height change), the line-height used for the text in the menu items in TypeaheadSearch is @line-height-small, which corresponds to 22px (assuming a base font size of 14px, which is what Vector uses). Before the change, it was 19.25px. The Figma spec sheet says it should be 20px.

This increase in line-height means that the text next to the thumbnail (two lines) is now 2*22=44px tall, up from 2*19.25=38.5px. The thumbnail is 40px tall (or 42px, see below), so changing the height of the text from 38.5px to 44px means that now the text, not the thumbnail, is the tallest thing in the menu item and controls its height. This causes the overall height of the menu item to increase. In the Figma spec, these two heights match: the thumbnail is 40px tall, and the text is also 40px (2*20px).

The thumbnail in each menu is set to be 40x40px. On the Codex demo site, this 40x40 measurement includes the borders (so the image itself is 38x38, and its total size including borders is 40x40), because Vitepress sets box-sizing: border-box everywhere. However, our code doesn't set border-box itself, so it's missing in a MediaWiki environment. This means that in Vector, the thumbnail is 40x40 not including borders, and its total size is 42x42. The Figma spec has the former (40x40 including borders). The other styles assume that the width of the thumbnail is 40px (even though it's 42px), which leads to a misalignment in Vector:

image (8).png (524×1 px, 106 KB)

  • Change the line-height for the MenuItem text to @line-height-x-small(=20px)
  • Apply box-sizing: border-box to the thumbnail, so that its height is always 40px, even in Vector
  • Update Vector's styles where needed; for example, this calculation will need to be updated to no longer add the thumbnail borders

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald Transcript

Change 852967 had a related patch set uploaded (by Catrope; author: Anne Tomasevich):

[design/codex@main] MenuItem: Reset line-height to @line-height-x-small

https://gerrit.wikimedia.org/r/852967

Change 852967 merged by jenkins-bot:

[design/codex@main] MenuItem: Reset line-height to @line-height-x-small

https://gerrit.wikimedia.org/r/852967

Will try to work on this next week

Change 859597 had a related patch set uploaded (by Anne Tomasevich; author: Anne Tomasevich):

[mediawiki/core@master] Update Codex from v0.2.2 to v0.3.0

https://gerrit.wikimedia.org/r/859597

Change 859597 merged by jenkins-bot:

[mediawiki/core@master] Update Codex from v0.2.2 to v0.3.0

https://gerrit.wikimedia.org/r/859597

AnneT set the point value for this task to 3.Dec 5 2022, 5:18 PM

Change 869274 had a related patch set uploaded (by VolkerE; author: Catrope):

[design/codex@main] build, tokens, styles: Introduce simple stylesheet unit transform

https://gerrit.wikimedia.org/r/869274

@Jdlrobson @Jdrewniak @bwang @nray @alexhollender_WMF With the last big tokens blocker on the way out – the relative unit sizing of certain token categories – we're also aiming at changing Icons and Thumbnails to relative sizes (ems) in order to be following user text zoom setting.
In order to not disrupt 100% rendering too much, we've added minimum width and height to the both components (scaling up is our accessibility prio), there's still one specific change, the Thumbnail (in Vector) will be 40x40px and not 42x42px, which is following design spec.

BeforeAfter
image.png (1×1 px, 246 KB)
image.png (1×1 px, 233 KB)

Test wiki created on Patch demo by ATomasevich (WMF) using patch(es) linked to this task:
https://patchdemo.wmflabs.org/wikis/bf2faf093a/w

Also created a Codex documentation site:
https://patchdemo.wmflabs.org/wikis/bf2faf093a/w/build/codex/docs

Test wiki on Patch demo by ATomasevich (WMF) using patch(es) linked to this task was deleted:

https://patchdemo.wmflabs.org/wikis/bf2faf093a/w/

Test wiki created on Patch demo by ATomasevich (WMF) using patch(es) linked to this task:
https://patchdemo.wmflabs.org/wikis/595f8966a0/w

Also created a Codex documentation site:
https://patchdemo.wmflabs.org/wikis/595f8966a0/w/build/codex/docs

Test wiki on Patch demo by ATomasevich (WMF) using patch(es) linked to this task was deleted:

https://patchdemo.wmflabs.org/wikis/595f8966a0/w/

Test wiki created on Patch demo by ATomasevich (WMF) using patch(es) linked to this task:
https://patchdemo.wmflabs.org/wikis/71dd00c269/w

Also created a Codex documentation site:
https://patchdemo.wmflabs.org/wikis/71dd00c269/w/build/codex/docs

ldelench_wmf subscribed.

Moving to sprint board since it looks like this is in progress - please edit if that's not the case.

Change 869274 merged by jenkins-bot:

[design/codex@main] build, tokens, styles: Introduce simple stylesheet unit transform

https://gerrit.wikimedia.org/r/869274

Test wiki on Patch demo by ATomasevich (WMF) using patch(es) linked to this task was deleted:

https://patchdemo.wmflabs.org/wikis/71dd00c269/w/

Change 884134 had a related patch set uploaded (by Catrope; author: Catrope):

[mediawiki/skins/Vector@master] search: Adjust SearchBoxLoader for border-box change in Codex

https://gerrit.wikimedia.org/r/884134

Change 869852 had a related patch set uploaded (by Catrope; author: Catrope):

[mediawiki/core@master] Update Codex from v0.4.3 to v0.5.0

https://gerrit.wikimedia.org/r/869852

Change 885450 had a related patch set uploaded (by Catrope; author: Catrope):

[mediawiki/core@master] Update Codex from v0.4.3 to v0.5.0

https://gerrit.wikimedia.org/r/885450

Change 885450 merged by jenkins-bot:

[mediawiki/core@master] Update Codex from v0.4.3 to v0.5.0

https://gerrit.wikimedia.org/r/885450

Change 884134 merged by jenkins-bot:

[mediawiki/skins/Vector@master] search: Adjust SearchBoxLoader for border-box change in Codex

https://gerrit.wikimedia.org/r/884134

Untagging as web team are tracking QA of this ticket as part ofT328053
Let me know if I've misunderstood.

egardner subscribed.

If web team is tracking the QA of this task separately, then I'm going ahead and closing this task since the Codex part of this work has already been released.