Skip to content

Adopt semantic color roles for the design system - #85

Merged
wesbillman merged 20 commits into
mainfrom
codex/block-ui-foundations
Sep 22, 2026
Merged

wesbillman merged 20 commits into
mainfrom
codex/block-ui-foundations

Conversation

@mahanti

@mahanti mahanti commented Sep 17, 2026

Copy link
Copy Markdown
Contributor

Buzz gets shared semantic colors for surfaces, text, borders and controls. Components choose a role by its purpose; the palette supplies light and dark values. Existing names remain as compatibility aliases while the later PRs migrate callers. Base UI remains the behavior layer, with public fonts and dependencies.

This is stack 1/4, targeting main after #73 merged. #86 builds controls, #87 adds compositions, and #88 adopts them across the app. Only #85 was updated in this integration; #86–88 were not restacked.

Current main integration (2026-09-22)

Carl, an automated reviewer, commenting via Wes’s GitHub account.

Head 400118a0ba578321f9d2eb4c347057cfe052feea merges main 9fce7a73fcf644de089ff556b60e8ccd7f036863 into previous head a8deea467f5d6c5700d61f7aa76fdce61814301a, without rewriting history. GitHub now reports MERGEABLE (no file conflicts), but the PR remains blocked by its existing changes-requested review and pending fresh CI.

  • Resolved five overlapping files: MentionCompletion, DESIGN.md, typography CSS, token registry, and viewer tests. Kept main’s local-agent enrollment/identity shapes and pinned typography provenance alongside Adopt semantic color roles for the design system #85’s semantic-color contract. Main’s button sizing, model-picker opening repair/browser regression, and deterministic signer test are byte-identical to main. Both independent bounded source-review lanes are clear; this is not approval.
  • Validation on this exact head/tree: all 1,989 Vitest tests in 189 files pass (17.08s wall, 84.71s summed test execution); 43 design tests pass; app/viewer production builds pass; all 28 viewer checks pass in Chromium/WebKit (19.5s). Normal hooks passed formatting, app/viewer typechecks, 18 selected unit tests, and all design guards. The pre-commit hook made no changes; pre-commit design tests/build used identical tree 349089dc5b43339f8207930ef0a84808da6fd299. Hosted DCO Check passed at the pushed head.
  • Browser cases added/removed by this integration: 0/0. No assertions, retries, or timeouts were weakened. The full existing viewer file ran with a temporary viewer-only port override (1443 → 1543), because another worktree owns 1443; the other server was untouched. An initial setup attempt also needed the viewer rebuild after the app build cleared dist; neither setup failure reached test execution.

Fresh hosted CI is still pending at handoff. Broad native/tool-integration and whole-app browser suites are left to CI; local viewer evidence is not desktop GUI or assembled-stack acceptance. No approval, review dismissal, merge, or downstream restacking was performed. Earlier validation below remains attributed to its historical snapshot.

Review fixes:

  • Error and warning boundaries now clear the documented 3:1 non-text target on base, panel, inset and popover surfaces in both themes. Only these semantic mappings change: danger uses red-9; warning uses amber-11 in light and amber-9 in dark. The lowest checked ratios are 3.228:1 and 4.046:1, respectively.
  • The contrast guard measures these boundary pairings and warning/success/accent text against their semantic fills, not just the underlying palette. Mutation tests reject the previous low-contrast mappings and a fill change that previously escaped the guard.
  • The design guide and color viewer consistently tell authors to use semantic roles. Obsolete guidance to write palette steps directly is removed.

An inherited channel-menu test race also surfaced in CI. The test now waits for menu closure, reopened menu focus, and restored trigger focus before advancing. No timeout, product behavior, or assertion was weakened.

Historical validation at 27dd5d0:

  • All hosted checks pass: JavaScript, native Rust/tool integration, Windows notifications, browser measurements, all four Chromium/WebKit journey shards, security checks, DCO and CI required.
  • Local formatting, app/viewer typechecks and design guards pass; 13 focused token/contrast tests, 43 design tests and all 1,801 unit tests pass.
  • App/viewer production builds and all 28 viewer browser checks pass in Chromium and WebKit. No browser scenarios were added or removed by this refresh.

The normal hooks remain enabled. An earlier local run hit the existing five-second app composition timeout during a competing build; isolated and unchanged full-hook runs passed afterward. Native behavior is inherited unchanged from #73. Desktop GUI acceptance of the assembled stack remains separate. Existing requested-change reviews need reviewer re-review; no review was dismissed.

Previous base refresh: a8deea467f5d6c5700d61f7aa76fdce61814301a merges #73 959aae7, including current main’s presence, development notification pause and Dock unread indicator. This merge was conflict-free and adds no new foundation behavior. Normal hooks passed app/viewer typechecks, design guards and all 1,865 unit tests in 180 files (17.81s wall, 107.50s summed execution). DCO and diff checks pass. All 12 hosted checks pass at this new head, including browser/native checks, measurements, security and DCO. The earlier local browser evidence above remains attributed to its stated snapshot.

Signed-off-by: Arjun Mahanti <arjun@squareup.com>
Signed-off-by: Arjun Mahanti <arjun@squareup.com>
Signed-off-by: Arjun Mahanti <arjun@squareup.com>
Signed-off-by: Arjun Mahanti <arjun@squareup.com>
Signed-off-by: Arjun Mahanti <arjun@squareup.com>
Signed-off-by: Arjun Mahanti <arjun@squareup.com>
Signed-off-by: Arjun Mahanti <arjun@squareup.com>
Signed-off-by: Arjun Mahanti <arjun@squareup.com>
Signed-off-by: Arjun Mahanti <arjun@squareup.com>
Signed-off-by: Arjun Mahanti <arjun@squareup.com>
Signed-off-by: Arjun Mahanti <arjun@squareup.com>
Signed-off-by: Arjun Mahanti <arjun@squareup.com>

@wesbillman wesbillman left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Carl, an automated reviewer, commenting via Wes’s GitHub account.

Changes requested: one P2 foundation-contract defect

Reviewed head f3c9be6ce79decace5c384366710bd69d301ca2e against exact stacked base 0e4b4d875b0c6190df9dae535d5aebfc52a16cd7 (codex/design-system-adoption-a, PR #73), not main.

P2: Give the new invalid/warning boundary roles sufficient non-text contrast. The inline finding covers newly exported roles advertised as control/state boundaries. Their current values fall below the documented 3:1 target. There is no migrated product consumer of these roles in this snapshot, so this is a defective new foundation contract, not a claim that this PR already breaks an existing screen. Fix the role mappings and add focused pairing coverage; no component migration or palette-wide redesign is needed.

Non-blocking notes

  • Reconcile the new semantic-only guidance with DESIGN.md:154-258,339-340 and the viewer's ColorPage.tsx:223-225, ColorTablePage.tsx:169-173, and OverviewPage.tsx:32-34, which still direct authors to use palette ramps. Those instructions now contradict the policy this PR introduces.
  • Bind warning/success/accent contrast checks to the new semantic fills as well as legacy palette consumers. check-contrast.mjs:140-145 still measures fixed palette pairs. In an isolated mutation, retargeting --affordance-warning from amber-3 to amber-9 left the contrast check green even though the dark warning-text pair fell to APCA Lc 15.4. Current unmutated text pairings pass; this coverage gap is not an additional current UI defect. The generic unresolved-reference skip predates this PR and is not attributed to it.

Validation and scope

  • Independently ran the changed color and contrast guards and git diff --check at the reviewed, clean head.
  • Compiled the actual host and viewer CSS with pinned Tailwind/PostCSS and probed Chromium and WebKit: all 36 new semantic utilities resolve to their advertised variables across light → dark → light; all 186 pre-existing token properties have identical computed values versus the exact base in both modes and both CSS entry points.
  • Independently checked border ratios with the repository's WCAG implementation. Complementary source review covered compatibility aliases, host appearance/backdrop ownership and contrast guards.
  • Hosted CI run 35222499015 is successful and reports this exact head; broad CI-equivalent suites were not rerun locally. Browser probes are focused CSS evidence, not full-app, enlarged-text, layout, native packaging or assembled-stack acceptance.
  • PR #73 remains open. This PR's body calls it a draft, but GitHub currently reports isDraft=false; reconcile that readiness statement separately. No approval or merge performed.

Comment thread src/shared/design-system/styles/tokens.css Outdated
Signed-off-by: Arjun Mahanti <arjun@squareup.com>
Signed-off-by: Arjun Mahanti <arjun@squareup.com>
Signed-off-by: Arjun Mahanti <arjun@squareup.com>
Signed-off-by: Arjun Mahanti <arjun@squareup.com>
Signed-off-by: Arjun Mahanti <arjun@squareup.com>
Signed-off-by: Arjun Mahanti <arjun@squareup.com>
…tings

Signed-off-by: Arjun Mahanti <arjun@squareup.com>

@wesbillman wesbillman left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Carl, an automated reviewer, commenting via Wes’s GitHub account.

Review clear at this head; no new blocking findings

Reviewed a8deea467f5d6c5700d61f7aa76fdce61814301a against exact stacked base 959aae7954002dcce0bdcb18eb2ef6dc20a13113 (PR #73), not main. This is a COMMENTED re-review, not approval or assembled-stack acceptance.

  • The earlier danger/warning boundary blocker is resolved. Independent WCAG 2 arithmetic over the current literal values verifies all 16 documented pairings: both roles against base, panel, inset and popover in both modes. Minimum ratios are 3.228:1 for danger and 4.046:1 for warning, above the 3:1 requirement (tokens.css:502–503,542–543; DESIGN.md:241–247).
  • The foundation contract is consistent. All 36 semantic roles have matching light/dark definitions, registry mappings and theme exports. Compatibility aliases retain their prior values; the host remains the appearance owner. Semantic-role guidance is aligned across the changed docs/viewer surfaces. The contrast mutation fixtures exercise the real checker and are included by root Vitest/CI.
  • One optional guard clarification: scripts/design-system/check-color.mjs:319–336 says each role must declare both modes, but its dark lookup falls back to the light declaration. All current new roles explicitly declare both modes, so this is not a defect in the values under review. Clarify that inheritance is allowed, or distinguish the new-role set from inherited aliases if explicit declarations are intended to be enforced.

Evidence and limits

Two complementary source-only lanes completed on a verified bare Blox object store. Existing CI reports all 12 checks successful for this exact head. No repository code was checked out, installed, built, tested, imported or executed during this review; no new rendered-pixel or desktop acceptance is claimed. Boundary arithmetic was independently derived; APCA was not independently reimplemented.

The fixed-height settings-button finding on the stacked PR #73 base is inherited, not introduced by #85, and remains a separate stack concern. This review does not dismiss older reviews, approve, merge, or certify the later consumer-migration PRs.

Base automatically changed from codex/design-system-adoption-a to main September 22, 2026 02:05
Preserve main's mention enrollment, typography provenance and sizing/interaction repairs alongside the semantic color contract. Resolve the five overlapping files without restacking downstream branches.

Signed-off-by: Carl <32a2e2c9d428ee08902cab75d956da2c1d235a22d4766b0dd4138bf6e2e5db1d@buzz.block.builderlab.xyz>
@wesbillman

Copy link
Copy Markdown
Collaborator

Carl, an automated reviewer, commenting via Wes’s GitHub account.

Resolved the five merge conflicts and pushed 400118a0ba578321f9d2eb4c347057cfe052feea, merging main 9fce7a73fcf644de089ff556b60e8ccd7f036863 without rewriting history. #85 now has no file conflicts; #86–88 were not changed.

The integration preserves main’s mention enrollment/agent shapes, pinned typography provenance, #73 button sizing, and picker/signer repairs, alongside #85’s semantic colors. Both bounded independent source-review lanes are clear.

Validation: 1,989/1,989 full Vitest tests, 43/43 design tests, 28/28 Chromium/WebKit viewer checks, app/viewer production builds, normal hooks and hosted DCO passed. Browser cases added/removed: 0/0. The PR description records exact snapshot and local port/setup details.

Fresh CI is pending: https://github.com/block/buzz-app/actions/runs/35679190661. The existing changes-requested gate remains; no approval, dismissal, or merge was performed. Native/whole-app CI and desktop GUI acceptance are not claimed by these local checks.

@wesbillman
wesbillman merged commit 4ac257e into main Sep 22, 2026
12 checks passed
@wesbillman
wesbillman deleted the codex/block-ui-foundations branch September 22, 2026 02:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants