[TEST] Facelift on Vibe 4 - #3467
Draft
Mordech wants to merge 29 commits into
Draft
Conversation
Squashed net effect of version3-facelift-exploration (formerly PR #3464) rebuilt on current master, so the package-separation moves are respected: the Label, Checkbox and ButtonGroup restyles land in packages/components/ rather than at their old packages/core/src/components/ paths. facelift/v3 is the canonical implementation of the facelift; this branch is its mechanical derivative. Propagate changes with scripts/sync-facelift.sh in the facelift-hackathon-aug26 cluster repo -- do not hand-author here. The Ported-from trailers below record the facelift/v3 commits whose effect this baseline already contains, so the sync script does not replay them. Ported-from: 217e49f Ported-from: 7f34e0a Ported-from: fcc4a84 Ported-from: ec1966f Ported-from: 6457376 Ported-from: 4381408 Ported-from: a182fac Ported-from: 7771efc Ported-from: 6584e4f Ported-from: 39db2c7 Ported-from: c89aae2 Ported-from: 2e40771 Ported-from: df8fee3 Ported-from: 38aa504
ResizeObserver.observe() always fires one callback immediately, and the observer effect depended on updateSelectedIndicator, so it re-subscribed on every tab change. One frame into the glide that callback ran the non-animated path, which sets `transition: none` and forces a reflow - snapping the indicator to its target and killing the animation. - remember the last applied left/width and make a non-animated update a no-op when the geometry has not actually changed, so the observer's initial callback cannot interrupt an in-flight transition - hold the updater in a ref so the observer stays subscribed across tab changes instead of tearing down and re-observing each time (cherry picked from commit 3d4c340) Ported-from: 3d4c340
Adds six semantic surfaces and their paired text colours, per the Facelift "surface colors" spec (Figma 630:106200): --surface-primary / -positive / -negative / -warning / -neutral / -info --text-on-surface-* for each The Figma layers are the base colour at 60% alpha, so the surfaces are built with color-mix() over each theme's existing token rather than hard-coded hexes. Overriding e.g. --warning-color-selected now flows into --surface-warning automatically, and one value composites correctly over light, dark and black. Verified against the spec's flattened values: primary #e0efff, warning within 6/255 (the designer's flatten wasn't precisely 60% of that base). Wiring: - Colors.ts maps the six semantics 1:1 onto the surfaces (primary no longer borrows --ui-background-color, neutral is a real semantic rather than an alias of info) and gains getOnSurfaceTextColor() - Chips pairs its JS-driven fill with the matching text colour; content colors have no pairing and keep the default text colour - Label kindFill uses the surface + on-surface text for every semantic - AttentionBox (legacy and next) fills switch to the surfaces; their copy stays --primary-text-color, as the design keeps it neutral - the tokens are documented in colorsMap and listed in kitchen-sink's panel Facelift's primary surface is indigo (#d4daff at 60% → #e5e9ff). --surface-info stays the informational blue in every theme, which is the point of having it once primary moves to indigo. Dark themes follow their own bright base semantics for on-surface text, since the spec only covers light. kitchen-sink no longer polyfills the surfaces - the package owns them now. Its Original-pane simulation keeps working via var() fallbacks to the pre-facelift tokens. (cherry picked from commit 9b8fdfd) Ported-from: 9b8fdfd
The close control was pinned to --primary-text-color, so it stayed dark while the chip's text moved to --text-on-surface-* for semantic colors. It now uses currentColor and inherits like the leading/trailing icons. The override is spelled `.icon.close` (three classes) on purpose: the close control is an IconButton, and its own .kindTertiary.colorOnPrimaryColor rule sets --text-color-on-primary at two-class specificity, which would otherwise win or lose on stylesheet order and could render a white icon on a pale surface. AttentionBox icons (legacy and next) likewise switch to currentColor so they track the box's text rather than pinning --primary-text-color. (cherry picked from commit e906624) Ported-from: e906624
Root cause, replacing my earlier specificity workaround:
Text renders its own colour class - the label came out as
`typography primary ...`, i.e. --primary-text-color pinned on the element.
A declaration on the element always beats inheritance, so the label ignored
the chip's inline --text-on-surface-* while the icons, being currentColor,
picked it up. Label and icons therefore disagreed. No amount of specificity
on the chip's own rules could fix that, because the competing declaration was
on a different element.
- the label passes color="inherit", so it follows the chip's colour
- the close IconButton drops color="on-primary-color", a leftover from when
chips had saturated fills; it forced --text-color-on-primary (white) at
two-class specificity, tying with `.chips .icon` and leaving the outcome to
stylesheet order. Without it the button only declares
`.kindTertiary { color: --primary-text-color }` at one class, which
`.chips .icon { color: currentColor }` beats outright
- reverted the `.icon.close` three-class escalation, no longer needed
Core: 158 files / 1476 tests pass, stylelint unchanged at 100 pre-existing.
(cherry picked from commit fa4caf5)
Ported-from: fa4caf5
Updated rule: each semantic surface is its own colour at 12%, rather than 60% of the *-color-selected token. --surface-primary: color-mix(in srgb, var(--primary-color) 12%, transparent); --surface-positive: color-mix(in srgb, var(--positive-color) 12%, transparent); --surface-negative: color-mix(in srgb, var(--negative-color) 12%, transparent); --surface-warning: color-mix(in srgb, var(--warning-color) 12%, transparent); --surface-neutral: color-mix(in srgb, var(--primary-text-color) 12%, transparent); --surface-info: color-mix(in srgb, var(--info-color) 12%, transparent); The formula is now byte-identical in all seven themes: each theme's own base colour flows through, so facelift's indigo --primary-color yields an indigo surface with no per-theme literal. The #d4daff hex the previous rule needed is gone. Adds --info-color (#0073ea) to every theme. v3 only had --info-color-selected, and `info` needs a base of its own so it can stay blue when the brand primary moves to indigo. Two derivations are mine, not from the spec, and worth a designer check: - neutral has no semantic colour, so it derives from --primary-text-color, which flips correctly between light and dark themes - --info-color is the classic monday blue (cherry picked from commit fcd1af0) Ported-from: fcd1af0
Chips resolved its fill in JS and wrote it to `style`, which is why consumers needed !important to recolour a chip: nothing but an important author declaration outranks an inline style. The fill was inline because hover was modelled in React state (isHovered + useSetFocus), so the value changed per render. Semantic colors now use the same `color-*` class pattern Label already has, reading --surface-* and --text-on-surface-*. The ~40 content colors can't each have a class, so they pass their palette in as a --chips-surface custom property; either way `background-color` itself stays in CSS, so hover is a plain :hover/:focus-within rule. That lets the JS hover machinery go entirely - isHovered, both mouse handlers and the useSetFocus call existed only to recompute the background, so this also drops a re-render on every hover. Content colors deliberately get no --chips-surface-hover: getElementColor returns early for them and ignores the hover palette, because no --color-*-selected-hover token exists. The previous JS hover branch produced the resting colour for them too, so behaviour is unchanged. A new test pins this down. kitchen-sink's chips-theme-* !important fills are deleted - the gallery already passes `color`, so the component handles it. Two of them had also drifted: -primary forced --ui-background-color and -info forced --surface-primary. (cherry picked from commit ba55ec3) Ported-from: ba55ec3
--primary-color #2b06e5 -> #3543fc --brand-color #2b06e5 -> #3543fc (follows primary) --positive-color #00854d -> #166d30 --negative-color #cd0026 (was #d83a52) --warning-color unchanged, marked UNTOUCHED in the design --primary-text-color unchanged; the swatch is bound to the existing token Applied to the three facelift themes only; the standard light/dark/black/hacker themes keep the monday palette. All six surfaces follow automatically, since they are color-mix of these at 12% - no surface token needed editing. Flattened on white the facelift surfaces are now primary #e7e8ff, positive #e3ede6, negative #f9e0e5, warning #fff9e0. --text-on-surface-primary was the literal #3543fc, which is now exactly --primary-color, so it derives from it instead of duplicating the hex. (cherry picked from commit d8e9c81) Ported-from: d8e9c81
Palette experiments belong in the consumer override, not the design system: mf-topbar's src/styles/themes/warmth.css can retune several themes at once without republishing Vibe. So --primary-color, --brand-color, --positive-color and --negative-color go back to their previous facelift values, and tokens.json is byte-identical to before the palette commit. Kept, because it is structure rather than a colour choice: --text-on-surface-primary stays var(--primary-color) instead of a duplicated hex, so it follows whatever primary the theme or an override sets. The surface work itself is unaffected: --surface-* are color-mix of these bases at 12%, so they track the reverted values automatically. (cherry picked from commit 760db57) Ported-from: 760db57
The palette lives here rather than in the design system, so several themes can be retuned without republishing Vibe - the same pattern as mf-topbar's src/styles/themes/warmth.css. styles.css sets only the base colours for the three facelift theme classes (Figma 684:115861 PRIMARIES: #3543fc / #166d30 / #cd0026; warning and --primary-text-color untouched). Everything derived follows on its own: --surface-* are color-mix of these at 12%, and --text-on-surface-primary is var(--primary-color). The selectors are qualified with `body` deliberately. AppThemeShell puts the theme class on <body>, and the package defines .facelift-*-app-theme at the same one-class specificity; in dev nothing is treeshaken and the package CSS is injected after styles.css, so an unqualified selector would lose the tie on source order. screens/styles/globals.css carries the same primaries for the Glaze screens. (cherry picked from commit ec9f997) Ported-from: ec9f997
…mth.css
Replaces the primaries-only block with a port of
mf-topbar/src/styles/themes/warmth.css, so the kitchen sink and mf-topbar
render the same palette instead of two hand-maintained subsets. Verified
token-for-token: 64 light, 39 dark, 39 black, identical names and values.
Only the selectors differ. mf-topbar puts `.warmth` on <html> and leans on the
host's light/dark/black class on <body>; the kitchen sink's AppThemeShell puts
`facelift-<mode>-app-theme` on <body>, so:
:root.warmth .light-app-theme, :root.warmth .default-app-theme
-> body.facelift-light-app-theme
They stay qualified with `body` because the style package defines
`.facelift-*-app-theme` at the same one-class specificity, and in dev its CSS is
injected after this file, so an unqualified selector would lose on source order.
Dropped from the port: topbar's `div[class*='useLocalDevMf']` rule, which hides
its own local-dev badge.
Also removed the `--ui-background-hover-color: #dfdbd8` one-off from styles.css;
warmth.css sets that token (#e8e6e6 for light) and now owns it.
This brings surfaces, borders, icons and radius across, not just the primaries.
Anything derived still follows on its own: the six --surface-* are color-mix of
these bases at 12%, and --text-on-surface-primary is var(--primary-color).
(cherry picked from commit 5818e61)
Ported-from: 5818e61
The facelift baseline carried v3's spelling over, but v4's IconButton declares only `"aria-label"`, so the prop was dropped and the chip's remove button had no accessible name. Three DropdownNew multi-select tests asserted on it and failed. No facelift/v3 counterpart: v3's IconButton really does take `ariaLabel`. This is a v4-adaptation fix, so it carries no Ported-from trailer.
PR #3464 replaced v4's TextField with v3's file wholesale, reverting master's modernization: `icon` became `iconName`, the flat `iconLabel` / `secondaryIconLabel` props collapsed back into an `iconsNames` object, `aria-label` became `ariaLabel`, `tabIndex` became a string, and Icon's `type` / `size` became `iconType` / `iconSize`. None of those spellings exist on v4, so they were silently dropped: an `icon` prop rendered no icon at all, and the secondary icon rendered as a bogus `<fa-star size="16">` element rather than a `<span role="img">`. The checked-in snapshot had expected the correct output all along, which is why it is unchanged by this fix. Restores master's file and re-applies only the facelift's genuine addition on top: the `iconPosition` prop with its `inputWrapperIconLeft` and `iconContainerLeft` classes (the styles were already present in TextField.module.scss). No facelift/v3 counterpart — v3's TextField really does take these names. This is a v4-adaptation fix, so it carries no Ported-from trailer.
The baseline was squashed from a branch based on an older master, so its
snapshots predate changes master has since made. Regenerated on v4:
* exports — SegmentedControl is exported on master
* Menu, Tabs — the --spacing-medium to --space-16 token rename
* Toast — master's close-button aria-label and message span
* SplitButton— master's hasRightIcon class
* Chips, Tabs— the class-based semantic colors and selected indicator
from the ports in this branch
Regeneration only; no source changes.
Aligns the theme sources with mf-topbar's warmth.css, which is where this
palette was tuned:
* Semantic surfaces are mixed against --primary-background-color rather
than `transparent`, at 16% (neutral stays 12%). Transparent surfaces
let whatever sits behind a Chip or AttentionBox bleed through, so the
same token read differently depending on its container.
* --text-on-surface-* reference the semantic tokens (--positive-color,
--negative-color, --warning-color, --info-color, --link-color) instead
of hardcoded hexes, so they follow the theme. Primary uses
--link-color: in the dark themes --primary-color is too dark to read
against its own surface.
* The dark and black palettes are lifted a step (dark
--primary-background-color #1a1a19 -> #2a2a28), with the kitchen-sink
theme overrides moved in lockstep.
These sources had drifted behind their own generated artifact:
packages/style/src/files/tokens.json was already committed with these
values, so `generate-colors` is a no-op on this commit. This makes the
sources match what the tokens have been describing.
(cherry picked from commit 67660b4)
Ported-from: 67660b4
Generated consequence of the palette port; no hand-edited values. The generator is idempotent — running it twice yields an identical file.
The baseline squash reverted master's numeric border-radius tokens (#3435) and re-added seven --spacing-* vars that master had renamed to --space-*. Both are v3 spellings that do not exist on v4, so the stylelint-config's allowed-vars map pointed at tokens the CSS output no longer defines — 929 style tests could not pass, 20 of them failing on exactly those names. Restores master's border-radius.scss and props-to-allowed-vars.js, then re-adds the facelift's genuine addition on top: --border-radius-full, which Chips uses for its pill shape. The facelift's flat `--border-radius-small: 4px` and master's `var(--border-radius-4)` alias compute to the same value, so this is not a visual change. No facelift/v3 counterpart — v3 has no numeric radii and should not gain them. This is a v4-adaptation fix, so it carries no Ported-from trailer.
The baseline squash carried the pre-#3465 version of these three files, silently reverting master's `perf(Dialog): add positioningActive to gate Floating UI position tracking` (f6150b4). Without it a permanently mounted dialog runs Floating UI's autoUpdate tracking continuously. The facelift never touched any of the three — each was byte-identical to the commit master built #3465 on — so this restores master's version outright and loses nothing. 90 Dropdown and 45 Dialog tests pass. No facelift/v3 counterpart: #3465 exists only on the v4 line. This is a v4-adaptation fix, so it carries no Ported-from trailer.
1 task
Mordech
changed the title
Mordech
changed the title
Two changes, applied across light, dark, black, hacker and the three facelift themes rather than only the facelift ones, so a non-facelift theme does not get a surface treatment that disagrees with the rest. - Surfaces drop from 12%/16% to 8% and mix over --primary-background-color instead of `transparent`. A translucent surface picks up whatever sits behind it, so nesting one inside another compounded the tint; opaque surfaces render the same wherever they land. - --secondary-background-color aligns with --allgrey-background-color in the dark and black themes (#404660 -> #363a50, #424240 -> #2e2e2c), which were reading as a third distinct grey next to the new surfaces. The surface comments still claimed 12% and "keeps them derived" without saying what they now mix over — updated in all six themes. tokens.json regenerated from the sources (verified idempotent). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> (cherry picked from commit 2655dbd) Ported-from: 2655dbd
The port of 2655dbd kept v4's generated tokens.json to get past the conflict — generated artifacts are never hand-merged, since merging two derived files can yield a tree matching neither source. Regenerated from the ported theme sources instead; the result is byte-identical to v3's. No Ported-from trailer: this is the regeneration step, not a port. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
|
📦 Bundle Size Analysis Changed Components
Unchanged Components
📊 Summary:
|
Contributor
|
A new prerelease version of this PR has been published! 🎉 |
Contributor
|
A new prerelease version of this PR has been published! 🎉 |
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.
Replaces #3464.
What this is
The facelift design work, rebuilt on current
master. This branch is a mechanical derivative —facelift/v3is the canonical branch, and everything here is cherry-picked from it bysync-facelift.shin the cluster super-repo. Every commit carries aPorted-from: <sha>trailer naming itsfacelift/v3counterpart.facelift/v3is canonical because the consuming microfrontends are still on Vibe 3 and build against its3.x-alphaprereleases. So that is where the work is authored, and this branch follows.Why it replaces #3464 instead of continuing it
#3464 branched off
masterat16385e8db.masterhas since landed 19 package-extraction commits — mass file moves that pulledLabel,Checkbox,ButtonGroupand nine other components out ofpackages/core/src/components/intopackages/components/<name>/src/. Rebasing #3464 across those renames was not viable.This branch instead starts fresh from
masterand transports #3464's net effect in one baseline commit viagit merge --squash, whose ort strategy follows renames. (Agit diff | git applyseed was rejected:applydoes no rename detection for content edits at deleted paths, so it would have silently dropped the facelift styling forLabel,CheckboxandButtonGroup— the branch would still build and the PR would still look fine.)Consequence: ancestry with #3464 is severed, so
git blameon facelift lines points at the baseline commit rather than the original authorship. That is accepted — blame for this work belongs onfacelift/v3.Bugs found and fixed while rebuilding
The old baseline was squashed from a branch built on an older
master, so it carried stale copies of filesmasterhad since advanced — silently reverting them. Three real regressions came out of a systematic sweep:TextField— [TEST] Facelift on Vibe 4 — semantic surfaces, component restyle, kitchen-sink #3464 had replaced v4's file with v3's wholesale, soicon,iconLabel/secondaryIconLabelandaria-labelarrived under their v3 spellings (iconName,iconsNames,ariaLabel). React drops unknown props without warning, so aniconprop rendered no icon at all and the secondary icon rendered as a bogus<fa-star size="16">. Master's file is restored, with onlyiconPositionre-applied on top.Dialog/DropdownPopup— master'spositioningActiveperf gate (perf(Dialog): add positioningActive to gate Floating UI position tracking #3465) was absent entirely. Restored.border-radiustokens — the baseline had swapped master's numeric radii for--border-radius-fulland re-added seven--spacing-*vars that do not exist on v4 (they are--space-*here). This was failing 20 style tests. Master's tokens are restored with--border-radius-fulladded alongside.Verification
yarn lerna run build— 32/32 projects, exit 0@vibe/core1231 tests passed,@vibe/style929 passed,@vibe/dialog45 passed,Dropdown90 passedyarn lintandstylelintcleanDraft until reviewed — flag anything and I'll fix it on
facelift/v3so the fix propagates here.🤖 Generated with Claude Code