refactor(frontend): four text sizes, two greys, and a rule that keeps them - #2440
Merged
Merged
Conversation
added 5 commits
August 11, 2026 18:17
Measured before touching anything: six sizes across nine size-and-weight combinations on one screen, and the distinctions did not line up with meaning. A section heading differed from a caption by weight alone at the same 12px. One 14px step carried a control, a metric's name, a card's label AND the metric's own value, so a row read flat — the number looked like its label. The same thing, a metric's name, was near-black in one place and grey in another. Two pill sizes, 10px and 11px, sat next to each other. Five roles now, declared in one module so the next change picks a role instead of inventing a size. A name is ink because the reader scans by it; what is grey is context — medians, comparisons, units, the pills that qualify a value. Three sizes turned out not to be choices at all. The tooltip set none and inherited the root 16px, so the same explanation rendered at 16px in one place and 14px in another. 11px was hand-set in three files. The card figure stepped between 24px and 30px on a container query, so one role rendered at two sizes depending on how wide its card happened to be. The zone's subtitle reads "Personal metrics" rather than "Pick a person": the zone is the person's own numbers, not a chooser. Refs constructorfabric#2408 Signed-off-by: Alexey Panfilov <alexey.panfilov@constructor.tech>
Measured first, because the shape of the problem was not what it looked like. Twelve sizes were in use — eight scale steps plus four written by hand — and four weights. But the two smallest steps carried 301 of 356 usages: the noise was a tail of 55 spread over ten variants, not the body of the work. That tail is gone. Nine, ten and eleven pixels were three ways of saying "small" that no reader can tell apart. Titles were set at three different sizes on different screens; the same figure rendered at 24, 30 or 36 pixels depending on which screen it landed on, and `tabular-nums` beside it is the tell that all of them were the same role. Four sizes remain, one per rung. Text greys go from five to two. A fractional shade is a third level the reader has to rank against the other two, and nobody can — so "grey is skippable" stops holding and every level has to be read after all. Colour now says one thing. Ten raw amber literals went back to the warning token, several of them added by me earlier today; good and bad were already success and destructive, and hierarchy was already ink and grey. Nothing else is expressed in colour. Vendored primitives under components/ui are untouched: their sizing belongs to that library and `shadcn add` rewrites them. Signed-off-by: Alexey Panfilov <alexey.panfilov@constructor.tech>
…tte colours The sweep before this one is worth nothing on its own. The scale had already drifted to twelve sizes and five greys once, and not through carelessness — every one of them was a reasonable local choice by someone who could not see the sum. Without something that says no, it grows back, and the next person measures it again from scratch. Three refusals, each naming the alternative rather than just objecting: a size written by hand points at the roles in type-scale, a fractional grey points at the two that exist, and a raw palette colour points at the semantic tokens. Checked by writing a violating file and confirming the rule fires on it — a guard that silently matches nothing is worse than no guard, because it is believed. Signed-off-by: Alexey Panfilov <alexey.panfilov@constructor.tech>
…smaller Eight of the eleven large numbers still spelled their size out; only three went through the role. A role nothing routes through is a comment rather than a rule, and the change below would have moved three figures and left eight where they were — which is how the scale drifted the first time. With all of them routed, the size is one line: 24px rather than 30. At 30 the number was the card, and everything that makes it mean something — what it counts, what it is measured against, which way it moved — sat around it as small print. It only has to beat the label above it, and it does that comfortably. Signed-off-by: Alexey Panfilov <alexey.panfilov@constructor.tech>
Found by the rule two commits back rather than by reading: the day strip set its captions at 11px directly. Same step as every other caption now. Signed-off-by: Alexey Panfilov <alexey.panfilov@constructor.tech>
|
Warning Review limit reached
Next review available in: 36 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe frontend adds a shared typography scale and ESLint rules for consistent text and semantic colors. Portal views, dashboard widgets, metric views, and screens replace local styling with shared roles and standard utility classes. ChangesTypography standardization
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
src/frontend/src/lib/type-scale.ts (1)
30-66: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueCorrect the role count in the module documentation.
type-scale.tsexports seven roles, not five.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/frontend/src/lib/type-scale.ts` around lines 30 - 66, Update the module documentation in type-scale.ts to state that the type scale exports seven roles, keeping the existing role definitions and styling unchanged.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/frontend/eslint.config.js`:
- Around line 109-121: Update the text utility selectors in the ESLint
configuration to include `:` alongside whitespace in their matching prefixes,
ensuring variant-prefixed utilities are checked. Expand the raw-palette selector
to cover every disallowed color and both unnumbered `text-black` and
`text-white`, while preserving the existing rule messages.
In `@src/frontend/src/components/widgets/metric-views/section-metric-index.tsx`:
- Line 98: Update the value container in the section metric view so its dd
retains only layout classes, apply the shared figure or foreground role to the
current metric value, and apply TEXT_LABEL exclusively to the median cell.
In `@src/frontend/src/lib/type-scale.ts`:
- Line 2: Update the documentation comment in the type-scale module to
accurately state that it provides seven text roles, or remove the role count
while preserving the description.
---
Nitpick comments:
In `@src/frontend/src/lib/type-scale.ts`:
- Around line 30-66: Update the module documentation in type-scale.ts to state
that the type scale exports seven roles, keeping the existing role definitions
and styling unchanged.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: f5ff6aa9-535e-4187-82cf-714a3d2e7752
📒 Files selected for processing (38)
src/frontend/eslint.config.jssrc/frontend/src/components/metric-evidence-dialog.tsxsrc/frontend/src/components/portal/ai-cost-view.tsxsrc/frontend/src/components/portal/attention-list.tsxsrc/frontend/src/components/portal/context-pane.test.tsxsrc/frontend/src/components/portal/context-pane.tsxsrc/frontend/src/components/portal/domain-lens-view.tsxsrc/frontend/src/components/portal/employees-view.tsxsrc/frontend/src/components/portal/manage-view.tsxsrc/frontend/src/components/portal/metric-groups-view.tsxsrc/frontend/src/components/portal/person-header.tsxsrc/frontend/src/components/portal/single-group-view.tsxsrc/frontend/src/components/portal/team-state-view.tsxsrc/frontend/src/components/portal/zone-content.tsxsrc/frontend/src/components/sidebar-settings.tsxsrc/frontend/src/components/widgets/dashboard/dashboard-header.tsxsrc/frontend/src/components/widgets/dashboard/ic-needs-attention.tsxsrc/frontend/src/components/widgets/dashboard/kpi-tile.tsxsrc/frontend/src/components/widgets/dashboard/members-grid.tsxsrc/frontend/src/components/widgets/dashboard/metric-sublabel.tsxsrc/frontend/src/components/widgets/dashboard/person-coverage.tsxsrc/frontend/src/components/widgets/dashboard/sparkline.tsxsrc/frontend/src/components/widgets/dashboard/team-members-attention.tsxsrc/frontend/src/components/widgets/metric-help-tooltip.tsxsrc/frontend/src/components/widgets/metric-views/metric-activity.tsxsrc/frontend/src/components/widgets/metric-views/metric-group-card.tsxsrc/frontend/src/components/widgets/metric-views/metric-summary-card.tsxsrc/frontend/src/components/widgets/metric-views/peer-comparison.tsxsrc/frontend/src/components/widgets/metric-views/peer-mark.tsxsrc/frontend/src/components/widgets/metric-views/peer-story.tsxsrc/frontend/src/components/widgets/metric-views/section-metric-index.tsxsrc/frontend/src/components/widgets/metric-views/team-metric-group-card.tsxsrc/frontend/src/components/widgets/period-selector-bar.tsxsrc/frontend/src/lib/type-scale.tssrc/frontend/src/screens/metric-definitions.tsxsrc/frontend/src/screens/metrics-console.tsxsrc/frontend/src/screens/query-console.tsxsrc/frontend/src/screens/whats-new.tsx
| @@ -0,0 +1,66 @@ | |||
| /** | |||
| * The five text roles a person-facing screen is allowed to use. | |||
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Correct the documented role count.
Line 2 says that the module has five text roles. The module exports seven roles. Change the count to seven, or remove the count.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/frontend/src/lib/type-scale.ts` at line 2, Update the documentation
comment in the type-scale module to accurately state that it provides seven text
roles, or remove the role count while preserving the description.
…a value Two findings, and the first one lands on the commit that claimed to have checked this. The selectors anchored on `(^|\s)`, so a variant prefix sitting directly against the utility walked straight past them: `hover:text-red-500`, `sm:text-[13px]` and `focus:text-muted-foreground/50` were all invisible to the rule. The palette list also held only the handful of families that happened to be in the tree, and `text-black` and `text-white` carry no number so they were missed for that reason alone. The earlier commit said a guard that silently matches nothing is worse than no guard, because it is believed — and then verified itself with a single probe that had no variant on it. The probe now covers each bypass and a legitimate line, so it demonstrates both halves: what the rule catches and what it leaves alone. Second: the metric index put the label role on the whole value container, which greyed a person's own value along with its median. The subject of the row and the context around it read the same, which is the one distinction this scale exists to make. The container keeps layout, the value takes ink, and only the median stays grey. Signed-off-by: Alexey Panfilov <alexey.panfilov@constructor.tech>
hello1101n
approved these changes
Aug 11, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The interface had drifted to twelve text sizes, five greys and four weights. None of that was decided — every one was a reasonable local choice by someone who could not see the sum, which is how this happens and why it will happen again without the last commit here.
What was actually wrong
Measured before touching anything, because the shape was not what it looked like. The two smallest steps carried 301 of 356 usages: the noise was a tail of 55 spread across ten variants, not the body of the work.
The tail had a pattern. Nine, ten and eleven pixels were three ways of saying "small" that no reader can tell apart. Page titles were set at three different sizes on different screens. The same figure rendered at 24, 30 or 36 pixels depending on which screen it landed on —
tabular-numssitting beside each one is the tell that they were the same role all along.Three of them turned out not to be choices at all. A tooltip set no size and inherited the root 16px, so the same explanation rendered at 16px in one place and 14px in another. A card figure stepped between two sizes on a container query, so one role rendered at two sizes depending on how wide its card happened to be.
Distinctions also failed to line up with meaning. A section heading differed from a caption by weight alone at the same size — two pixels of difference is not a distinction anybody sees. One step carried a control, a metric's name, a card's label AND the metric's own value, so a row read flat: the number looked like its label. And the same thing — a metric's name — was near-black in one place and grey in another.
What it is now
Four sizes, one role each: captions, body and names, titles, figures. Two greys. Colour says one thing — good and bad are
successanddestructive, attention without a verdict iswarning, and nothing else is expressed in colour at all. Ten raw palette literals went back to tokens, several of them added by me a few hours earlier.A name is ink because the reader scans by it. What is grey is context: medians, comparisons, units, the pills that qualify a value. Grey is what the eye may skip, so nothing that must be read is put there.
The commit that matters
The sweep is worth nothing on its own — it grows back, and the next person measures it from scratch. So a lint rule refuses hand-set sizes, fractional greys and raw palette colours, and each refusal names the alternative rather than just objecting.
It was checked by writing a violating file and confirming the rule fires on it. A guard that silently matches nothing is worse than no guard, because it is believed. It has already earned its place: the last commit here fixes a hand-set size the rule found rather than a reader did.
Also worth knowing: of eleven large numbers, only three went through the shared role — the other eight spelled their size out. A role nothing routes through is a comment, not a rule, and changing the figure size would have moved three of them and left eight behind. All eleven route through it now, which is why the figure could be reduced in one line.
Scope
Vendored primitives under
src/components/uiare untouched: their sizing belongs to that library andshadcn addrewrites them. The rule exempts them for the same reason.Checks
tsc -bclean, eslint clean, 1073 unit tests pass. Reviewed on screen at each step rather than only in the diff.Summary by CodeRabbit