Manager UI - Replace raw fontSize literals with typography variants - #4308
Manager UI - Replace raw fontSize literals with typography variants#4308agalin920 wants to merge 3 commits into
Conversation
Refs #4287. The exact-match slice only; the overline, ambiguous and no-variant slices are held pending a design decision.
Refs #4287. Rendered change: letter-spacing -0.56px -> 1px on ten headers. Box widths and heights are unchanged.
agalin920
changed the title
agalin920
changed the title
Coverage ReportOverall Coverage
Changed Files Coverage
|
QA Review —
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| fontSize: "12px", | ||
| typography: "body3", | ||
| // body3 carries display: inline-block; a subheader must stay full width. | ||
| display: "list-item", |
There was a problem hiding this comment.
🟡 display: "list-item" is the wrong semantic override for inline-block
display: list-item creates a marker box on the li; no bullet appears here only because the parent ul[role="listbox"] inherits list-style: none from MUI's Autocomplete styles — a detail this element now silently depends on. If the component is ever reused outside that ul, or if MUI's reset changes, a bullet will appear. display: "block" achieves the same full-width result without the marker-box side-effect and without the hidden dependency.
| display: "list-item", | |
| display: "block", |
| fontSize: "12px", | ||
| typography: "body3", | ||
| // body3 carries display: inline-block; a subheader must stay full width. | ||
| display: "list-item", |
There was a problem hiding this comment.
🟡 Same display: "list-item" fragility — should be display: "block"
Same reasoning as the RecentSearches header above: display: list-item works only because the parent ul suppresses the marker via list-style: none; display: "block" is the semantically correct and dependency-free fix.
| display: "list-item", | |
| display: "block", |
Code Review — ✅ No blockers · 🟡 2 warning(s) — see inline comments |
Negative QANo reproducible edge-case failures found on the surfaces this PR changes. Also checked and working correctly
|
3 participants
Refs #4287. Covers the sweep everywhere it can be made without changing a rendered value the design system does not already own; what is left is listed at the bottom and the issue stays open.
Two commits, deliberately separable.
c48906bd4— 26 literals, rendered-identical. 15 sites at14px/20pxbecomebody2, the two GlobalSearchListSubheaders becomebody3, and 9fontSize: "16px"declarations on widgetCardHeaderroots are deleted rather than converted — those roots have zero direct text nodes, so the declaration renders nothing. Verified as agetComputedStylebefore/after diff over the 112 text-bearing elements of the content-item widget rail: zero differences in font-size, line-height, letter-spacing, weight, display or box size.body3carriesdisplay: inline-block, which would collapse a sticky full-width subheader, sodisplayis pinned beside the variant.8d19d7826— the 10overlineheaders, a deliberate rendered change. The item-editor widget headers hand-rolledoverlineand lost its tracking; they were inheritingletter-spacing: -0.56pxfromCardHeader's defaulth5variant, notnormalas the issue assumes. Adopting the variant moves all ten toletter-spacing: 1px— measured, and it is the only property that changes: font-size stays 12px, line-height 32px, weight 400, colourrgb(16,24,40), and every box stays 287×32, so nothing reflows or wraps. Eight headers also pick uptext-transform: uppercasefrom the variant with no visible effect, their titles already being literal uppercase strings.Raw
fontSizeliterals insrc/go 63 → 27 across 24 files.What the 27 remaining occurrences are, and why none of them are in this PR. 16 are MUI icons or a
Skeletonand are out of scope by the issue's own Scope section — which also means its "18px×5 — no variant exists" and "28/20/32×5 →h3/h5/h2" rows should not be actioned at all; they would resize glyphs. 5 have no variant to adopt (10px/14px×2, and threeListItemsites measured at a rendered14/24, which is neitherbody214/20 norsubtitle214/22) and go to design underdocs/design-system.md§4. 4 sit on surfaces I could not reach to measure — two empty states and aLinkin the code editor's bottom drawer.The last 2 are two of the issue's own 19 "exact matches" and are deliberately left:
CompareDialog.tsx:337andPropertiesDialog.tsx:362set12px/18pxviasecondaryTypographyPropsbut, unlike the GlobalSearch pair, carry noletterSpacingtoday, so adoptingbody3would add 0.15px of tracking anddisplay: inline-block. Both render only in Analytics dialogs that need a connected GA property, so I could not measure the result and did not guess it. 17 of 19, not 19.Typecheck 0 errors / 0 in
src/, unchanged from the branch point.npm run build:devexits 0.cypress/e2e/content/actions.spec.js17/17 andcypress/e2e/search/search-bar.spec.js4/4.No
reviewerpass ran on either commit — the session instruction ruled out subagents, so this diff was reviewed only by its author.