From ada85ac8903777de37002a04e6ea2005e3ead784 Mon Sep 17 00:00:00 2001 From: Khoi Nguyen Date: Thu, 9 Jul 2026 13:59:32 +0700 Subject: [PATCH 1/6] ALFMOB-271: Source ThemedButton styling from semantic design tokens --- .../SharedUI/Theme/Buttons/ButtonTheme.swift | 45 +++++++------ .../SharedUI/Theme/Buttons/ThemedButton.swift | 4 +- .../SharedUITests/ButtonThemeTests.swift | 64 +++++++++++++++++++ 3 files changed, 92 insertions(+), 21 deletions(-) create mode 100644 Alfie/AlfieKit/Tests/SharedUITests/ButtonThemeTests.swift diff --git a/Alfie/AlfieKit/Sources/SharedUI/Theme/Buttons/ButtonTheme.swift b/Alfie/AlfieKit/Sources/SharedUI/Theme/Buttons/ButtonTheme.swift index 2af11607..68879281 100644 --- a/Alfie/AlfieKit/Sources/SharedUI/Theme/Buttons/ButtonTheme.swift +++ b/Alfie/AlfieKit/Sources/SharedUI/Theme/Buttons/ButtonTheme.swift @@ -28,52 +28,57 @@ public enum ButtonTheme: String, CaseIterable { var spec: ButtonThemeSpec { switch self { + // Default/disabled colors come from the semantic `Theme.button*` tokens; the pressed state + // has no semantic token, so it stays on `Primitives.Colours.*`. case .primary: return .init( - backgroundColor: Primitives.Colours.neutrals800, - backgroundDisabledColor: Primitives.Colours.neutrals100, + backgroundColor: Theme.buttonPrimaryBackgroundPrimaryDefault, + backgroundDisabledColor: Theme.buttonPrimaryBackgroundPrimaryDisabled, backgroundPressedColor: Primitives.Colours.neutrals500, - textColor: Primitives.Colours.neutrals0, - textDisabledColor: Primitives.Colours.neutrals400, + textColor: Theme.buttonPrimaryContentPrimaryDefault, + textDisabledColor: Theme.buttonPrimaryContentPrimaryDisabled, textPressedColor: Primitives.Colours.neutrals0, - borderColor: Primitives.Colours.neutrals800, - borderDisabledColor: Primitives.Colours.neutrals100, + borderColor: Theme.buttonPrimaryStrokePrimaryDefault, + borderDisabledColor: Theme.buttonPrimaryStrokePrimaryDisabled, borderPressedColor: Primitives.Colours.neutrals500 ) case .secondary: return .init( - backgroundColor: Primitives.Colours.neutrals0, - backgroundDisabledColor: Primitives.Colours.neutrals0, + backgroundColor: Theme.buttonSecondaryBackgroundSecondaryDefault, + backgroundDisabledColor: Theme.buttonSecondaryBackgroundSecondaryDisabled, backgroundPressedColor: Primitives.Colours.neutrals100, - textColor: Primitives.Colours.neutrals800, - textDisabledColor: Primitives.Colours.neutrals400, + textColor: Theme.buttonSecondaryContentSecondaryDefault, + textDisabledColor: Theme.buttonSecondaryContentSecondaryDisabled, textPressedColor: Primitives.Colours.neutrals500, - borderColor: Primitives.Colours.neutrals800, - borderDisabledColor: Primitives.Colours.neutrals400, + borderColor: Theme.buttonSecondaryStrokeSecondaryDefault, + borderDisabledColor: Theme.buttonSecondaryStrokeSecondaryDisabled, borderPressedColor: Primitives.Colours.neutrals500 ) case .tertiary: return .init( - backgroundColor: Primitives.Colours.neutrals0, - backgroundDisabledColor: Primitives.Colours.neutrals0, + backgroundColor: Theme.buttonTerciaryBackgroundTerciaryDefault, + backgroundDisabledColor: Theme.buttonTerciaryBackgroundTerciaryDisabled, backgroundPressedColor: Primitives.Colours.neutrals100, - textColor: Primitives.Colours.neutrals800, - textDisabledColor: Primitives.Colours.neutrals400, + textColor: Theme.buttonTerciaryContentTerciaryDefault, + textDisabledColor: Theme.buttonTerciaryContentTerciaryDisabled, textPressedColor: Primitives.Colours.neutrals500, - borderColor: Primitives.Colours.neutrals0, - borderDisabledColor: Primitives.Colours.neutrals0, + borderColor: Theme.buttonTerciaryStrokeTerciaryDefault, + borderDisabledColor: Theme.buttonTerciaryStrokeTerciaryDisabled, borderPressedColor: Primitives.Colours.neutrals100 ) + // Underline has no semantic button group → its text tracks the semantic `Theme.linkLink*` + // tokens (an underline button is a link); pressed text and the invisible background/border + // have no matching token and stay on `Primitives.Colours.*`. case .underline: return .init( backgroundColor: Primitives.Colours.neutrals0, backgroundDisabledColor: Primitives.Colours.neutrals0, backgroundPressedColor: Primitives.Colours.neutrals0, - textColor: Primitives.Colours.neutrals800, - textDisabledColor: Primitives.Colours.neutrals400, + textColor: Theme.linkLinkPrimaryDefault, + textDisabledColor: Theme.linkLinkPrimaryDisabled, textPressedColor: Primitives.Colours.neutrals500, borderColor: Primitives.Colours.neutrals0, borderDisabledColor: Primitives.Colours.neutrals0, diff --git a/Alfie/AlfieKit/Sources/SharedUI/Theme/Buttons/ThemedButton.swift b/Alfie/AlfieKit/Sources/SharedUI/Theme/Buttons/ThemedButton.swift index b567c308..c3e6d870 100644 --- a/Alfie/AlfieKit/Sources/SharedUI/Theme/Buttons/ThemedButton.swift +++ b/Alfie/AlfieKit/Sources/SharedUI/Theme/Buttons/ThemedButton.swift @@ -136,7 +136,8 @@ private enum Constants { static let horizontalPadding: CGFloat = 0 static let verticalPadding: CGFloat = -Primitives.Spacing.spacing8 static let cornerRadius: CGFloat = Sizing.radiusSoft - static let iconSize: CGFloat = 16 + static let iconSize: CGFloat = Sizing.iconsIconSmall + // No design token exists for button heights — literals retained. static let smallHeight: CGFloat = 36 static let mediumHeight: CGFloat = 44 static let bigHeight: CGFloat = 52 @@ -181,6 +182,7 @@ private struct ThemedButtonStyle: ButtonStyle { .overlay( ZStack { RoundedRectangle(cornerRadius: Constants.cornerRadius) + // No design token exists for the 1pt stroke width — literal retained. .stroke(borderColor(configuration), lineWidth: 1) if isLoading { diff --git a/Alfie/AlfieKit/Tests/SharedUITests/ButtonThemeTests.swift b/Alfie/AlfieKit/Tests/SharedUITests/ButtonThemeTests.swift new file mode 100644 index 00000000..b7d3cffd --- /dev/null +++ b/Alfie/AlfieKit/Tests/SharedUITests/ButtonThemeTests.swift @@ -0,0 +1,64 @@ +import SwiftUI +import XCTest +@testable import SharedUI + +/// Pins each `ButtonTheme` variant to the design tokens it must source colors from, so a future +/// token regeneration (or an accidental edit) that re-points a mapping is caught. Default/disabled +/// colors come from the semantic `Theme.button*` layer; the pressed state and the underline +/// background/border have no semantic token and intentionally stay on `Primitives.Colours.*`. +final class ButtonThemeTests: XCTestCase { + func test_primary_sourcesSemanticButtonTokens() { + let spec = ButtonTheme.primary.spec + XCTAssertEqual(spec.backgroundColor, Theme.buttonPrimaryBackgroundPrimaryDefault) + XCTAssertEqual(spec.backgroundDisabledColor, Theme.buttonPrimaryBackgroundPrimaryDisabled) + XCTAssertEqual(spec.textColor, Theme.buttonPrimaryContentPrimaryDefault) + XCTAssertEqual(spec.textDisabledColor, Theme.buttonPrimaryContentPrimaryDisabled) + XCTAssertEqual(spec.borderColor, Theme.buttonPrimaryStrokePrimaryDefault) + XCTAssertEqual(spec.borderDisabledColor, Theme.buttonPrimaryStrokePrimaryDisabled) + // Pressed: no semantic token — primitives retained. + XCTAssertEqual(spec.backgroundPressedColor, Primitives.Colours.neutrals500) + XCTAssertEqual(spec.textPressedColor, Primitives.Colours.neutrals0) + XCTAssertEqual(spec.borderPressedColor, Primitives.Colours.neutrals500) + } + + func test_secondary_sourcesSemanticButtonTokens() { + let spec = ButtonTheme.secondary.spec + XCTAssertEqual(spec.backgroundColor, Theme.buttonSecondaryBackgroundSecondaryDefault) + XCTAssertEqual(spec.backgroundDisabledColor, Theme.buttonSecondaryBackgroundSecondaryDisabled) + XCTAssertEqual(spec.textColor, Theme.buttonSecondaryContentSecondaryDefault) + XCTAssertEqual(spec.textDisabledColor, Theme.buttonSecondaryContentSecondaryDisabled) + XCTAssertEqual(spec.borderColor, Theme.buttonSecondaryStrokeSecondaryDefault) + XCTAssertEqual(spec.borderDisabledColor, Theme.buttonSecondaryStrokeSecondaryDisabled) + XCTAssertEqual(spec.backgroundPressedColor, Primitives.Colours.neutrals100) + XCTAssertEqual(spec.textPressedColor, Primitives.Colours.neutrals500) + XCTAssertEqual(spec.borderPressedColor, Primitives.Colours.neutrals500) + } + + func test_tertiary_sourcesSemanticButtonTokens() { + let spec = ButtonTheme.tertiary.spec + XCTAssertEqual(spec.backgroundColor, Theme.buttonTerciaryBackgroundTerciaryDefault) + XCTAssertEqual(spec.backgroundDisabledColor, Theme.buttonTerciaryBackgroundTerciaryDisabled) + XCTAssertEqual(spec.textColor, Theme.buttonTerciaryContentTerciaryDefault) + XCTAssertEqual(spec.textDisabledColor, Theme.buttonTerciaryContentTerciaryDisabled) + XCTAssertEqual(spec.borderColor, Theme.buttonTerciaryStrokeTerciaryDefault) + XCTAssertEqual(spec.borderDisabledColor, Theme.buttonTerciaryStrokeTerciaryDisabled) + XCTAssertEqual(spec.backgroundPressedColor, Primitives.Colours.neutrals100) + XCTAssertEqual(spec.textPressedColor, Primitives.Colours.neutrals500) + XCTAssertEqual(spec.borderPressedColor, Primitives.Colours.neutrals100) + } + + func test_underline_sourcesSemanticLinkTokensForText() { + let spec = ButtonTheme.underline.spec + // Underline has no semantic button group → text tracks the semantic link tokens. + XCTAssertEqual(spec.textColor, Theme.linkLinkPrimaryDefault) + XCTAssertEqual(spec.textDisabledColor, Theme.linkLinkPrimaryDisabled) + // Pressed text + the (invisible) background/border have no matching token — primitives retained. + XCTAssertEqual(spec.textPressedColor, Primitives.Colours.neutrals500) + XCTAssertEqual(spec.backgroundColor, Primitives.Colours.neutrals0) + XCTAssertEqual(spec.backgroundDisabledColor, Primitives.Colours.neutrals0) + XCTAssertEqual(spec.backgroundPressedColor, Primitives.Colours.neutrals0) + XCTAssertEqual(spec.borderColor, Primitives.Colours.neutrals0) + XCTAssertEqual(spec.borderDisabledColor, Primitives.Colours.neutrals0) + XCTAssertEqual(spec.borderPressedColor, Primitives.Colours.neutrals0) + } +} From d50e7c142529b15047c2064b317112e619389a73 Mon Sep 17 00:00:00 2001 From: Khoi Nguyen Date: Thu, 9 Jul 2026 14:00:34 +0700 Subject: [PATCH 2/6] ALFMOB-271: Add plan & decision log for ThemedButton token refactor --- .../_status.md | 23 +++++ .../grill.md | 25 +++++ .../phase-1-colors-semantic.md | 54 +++++++++++ .../phase-2-sizing-literals.md | 26 +++++ .../plan.md | 95 +++++++++++++++++++ .../scope.md | 38 ++++++++ 6 files changed, 261 insertions(+) create mode 100644 Docs/Plans/ALFMOB-271-refactor-themedbutton-tokens/_status.md create mode 100644 Docs/Plans/ALFMOB-271-refactor-themedbutton-tokens/grill.md create mode 100644 Docs/Plans/ALFMOB-271-refactor-themedbutton-tokens/phase-1-colors-semantic.md create mode 100644 Docs/Plans/ALFMOB-271-refactor-themedbutton-tokens/phase-2-sizing-literals.md create mode 100644 Docs/Plans/ALFMOB-271-refactor-themedbutton-tokens/plan.md create mode 100644 Docs/Plans/ALFMOB-271-refactor-themedbutton-tokens/scope.md diff --git a/Docs/Plans/ALFMOB-271-refactor-themedbutton-tokens/_status.md b/Docs/Plans/ALFMOB-271-refactor-themedbutton-tokens/_status.md new file mode 100644 index 00000000..d0d3cc5d --- /dev/null +++ b/Docs/Plans/ALFMOB-271-refactor-themedbutton-tokens/_status.md @@ -0,0 +1,23 @@ +# ALFMOB-271 — Refactor ThemedButton to use design tokens + +- **Ticket:** ALFMOB-271 (Story, Epic ALFMOB-264) — https://mindera.atlassian.net/browse/ALFMOB-271 +- **Base branch (PR target):** feat/ALFMOB-264-adopt-design-tokens +- **Work branch:** ALFMOB-271-refactor-themedbutton-tokens +- **High-rigor:** no + +## Summary +Update `ThemedButton` / `ButtonTheme` to read all styling (colors, typography, spacing, +corner radius, border) from generated design tokens instead of hardcoded references. +Keep the public `ThemedButton` API unchanged. Cover 4 styles × 3 sizes × states with snapshots. + +Files: `Alfie/AlfieKit/Sources/SharedUI/Theme/Buttons/{ThemedButton.swift,ButtonTheme.swift,ThemedButton+Extension.swift}` + +## Phase checklist +- [x] SCOUT → scope.md +- [x] PLAN → plan.md +- [x] GRILL → grill.md + hardened plan.md +- [x] APPROVAL gate ← approved 2026-07-09 +- [x] IMPLEMENT (ios-execute: verify ✅ + review APPROVED) +- [ ] COMMIT +- [ ] PR → feat/ALFMOB-264-adopt-design-tokens +- [ ] TICKET → In Review diff --git a/Docs/Plans/ALFMOB-271-refactor-themedbutton-tokens/grill.md b/Docs/Plans/ALFMOB-271-refactor-themedbutton-tokens/grill.md new file mode 100644 index 00000000..a408cf32 --- /dev/null +++ b/Docs/Plans/ALFMOB-271-refactor-themedbutton-tokens/grill.md @@ -0,0 +1,25 @@ +# Grill: Refactor ThemedButton to use design tokens +**Plan**: Docs/Plans/ALFMOB-271-refactor-themedbutton-tokens/plan.md **Ticket**: ALFMOB-271 **Date**: 2026-07-09 **Branch**: ALFMOB-271-refactor-themedbutton-tokens + +## Decisions +| # | Decision | Recommended | Chosen | Plan change | +|---|---|---|---|---| +| 1 | Semantic `Theme.button*` layer vs keep primitives | Adopt semantic | **Adopt semantic** | Approach + phase-1 mapping table point Primary/Secondary/Tertiary Default+Disabled at `Theme.button*` | +| 2 | Accept visual deltas from semantic values | Yes | **Yes** | Recorded as design-authoritative in Approach/Risks | +| 3 | Satisfy snapshot AC6 given disabled infra | Unit test now, defer snapshots | **Unit test now, defer** (no follow-up ticket) | AC6 reworded; snapshot re-enable stays Out of Scope | +| 4 | Underline color source (no semantic button group) | Keep primitives | **Use `Theme.linkLink*`** (text default/disabled) | Approach + phase-1: underline text→link tokens; pressed/bg/border→primitives | + +## Answered by the codebase (not asked) +- **Pressed state has no semantic token** → must stay on `Primitives.Colours.*`. `Theme+Generated.swift:9-32` exposes only `…Default`/`…Disabled` per button group; no pressed. (auto-resolved) +- **`iconSize` tokenization** → `Sizing.iconsIconSmall` == `Primitives.Spacing.spacing16` == 16, exact match. `Sizing+Generated.swift:9`. (auto-resolved) +- **Button heights & 1pt stroke have no token** → `Sizing+Generated.swift` has no 36/44/52 height or stroke-width token; remain documented literals. (auto-resolved) +- **Ticket premise stale** (`Colors.primary.mono900`) → that facade doesn't exist on this branch; colors already on `Primitives.Colours.*`. scope.md + `ButtonTheme.swift:31-81`. (auto-resolved) + +## Assumptions surfaced +- "Use generated tokens" was implicitly read as "primitive tokens"; the plan makes explicit the intent is the **semantic** `Theme.*` layer, and that this is a *deliberate visual change*, not a no-op. +- Underline text default/disabled numerically equal the link tokens **today** — mapping is zero-pixel now but couples underline to the design system's link color going forward (accepted). +- Snapshot suite being out of target membership is treated as a pre-existing repo condition, not this ticket's problem to fix. + +## Still open (owner) +- Repo-wide snapshot-suite re-enable (target membership + committed refs) — **Design/Platform**, tracked outside ALFMOB-271; not blocking this PR. +- Whether the accepted disabled-shade shifts need Design sign-off — **Design/PM**; low-risk (neutral shades, dev integration branch), proceeding without a hard gate. diff --git a/Docs/Plans/ALFMOB-271-refactor-themedbutton-tokens/phase-1-colors-semantic.md b/Docs/Plans/ALFMOB-271-refactor-themedbutton-tokens/phase-1-colors-semantic.md new file mode 100644 index 00000000..c93ed780 --- /dev/null +++ b/Docs/Plans/ALFMOB-271-refactor-themedbutton-tokens/phase-1-colors-semantic.md @@ -0,0 +1,54 @@ +## Phase 1: Colors → semantic token layer + +### Goal +`ButtonThemeSpec` sources its Default/Disabled colors from the semantic `Theme.button*` tokens +instead of raw `Primitives.Colours.neutrals*`, locked by a unit test. App/package builds & green. + +### Acceptance criteria +- [ ] Primary/Secondary/Tertiary Default + Disabled bg/text/border read from `Theme.button*` tokens. +- [ ] Underline text Default/Disabled read from `Theme.linkLinkPrimary*`; its pressed text + bg/border + stay `Primitives.Colours.*`. +- [ ] Pressed colors (all variants) remain `Primitives.Colours.*` with a one-line comment (no semantic token). +- [ ] `ButtonThemeTests` asserts each variant's spec fields equal the intended token constants. +- [ ] `./Alfie/scripts/verify.sh` green; Xcode previews render all 4 variants unchanged in structure. + +### Token mapping (from Theme+Generated.swift — DO NOT edit generated file) +| Variant | field | new token | +|---|---|---| +| primary | backgroundColor | `Theme.buttonPrimaryBackgroundPrimaryDefault` | +| primary | backgroundDisabledColor | `Theme.buttonPrimaryBackgroundPrimaryDisabled` | +| primary | textColor | `Theme.buttonPrimaryContentPrimaryDefault` | +| primary | textDisabledColor | `Theme.buttonPrimaryContentPrimaryDisabled` | +| primary | borderColor | `Theme.buttonPrimaryStrokePrimaryDefault` | +| primary | borderDisabledColor | `Theme.buttonPrimaryStrokePrimaryDisabled` | +| secondary | backgroundColor / Disabled | `Theme.buttonSecondaryBackgroundSecondaryDefault` / `…Disabled` | +| secondary | textColor / Disabled | `Theme.buttonSecondaryContentSecondaryDefault` / `…Disabled` | +| secondary | borderColor / Disabled | `Theme.buttonSecondaryStrokeSecondaryDefault` / `…Disabled` | +| tertiary | background/text/border Default+Disabled | `Theme.buttonTerciary…` (note generated spelling "Terciary") | +| underline | textColor | `Theme.linkLinkPrimaryDefault` | +| underline | textDisabledColor | `Theme.linkLinkPrimaryDisabled` | +| underline | textPressed + background/border (all states) | keep `Primitives.Colours.*` (no link pressed/bg/border token) | +| *pressed (all other variants)* | background/text/border Pressed | keep `Primitives.Colours.*` (no semantic token) | + +### Steps +1. **Re-point Primary/Secondary/Tertiary Default+Disabled colors** (file: `ButtonTheme.swift:31-68`, size: XS) — + swap the raw `Primitives.Colours.*` for the mapped `Theme.button*` constants per the table. + Leave the three `*PressedColor` fields on `Primitives.Colours.*`. Add a `// no semantic pressed token` + comment. Why: adopt the semantic layer (AC1) while preserving pressed behavior. +2. **Underline text → semantic link tokens** (file: `ButtonTheme.swift:70-81`, size: XS) — set + `textColor: Theme.linkLinkPrimaryDefault`, `textDisabledColor: Theme.linkLinkPrimaryDisabled`; + keep `textPressedColor` + all bg/border on `Primitives.Colours.*`. Add comment + `// underline: no semantic button group → link tokens for text; primitives for pressed/bg/border`. +3. **Add token-mapping unit test** (file: `Alfie/AlfieKit/Tests/SharedUITests/ButtonThemeTests.swift`, size: S) — + new `XCTestCase` (or Swift Testing `@Test`, match SharedUITests convention). For each variant assert + `ButtonTheme..spec. == `. `ButtonThemeSpec` is + `internal` and tests are in the same package → `@testable import SharedUI` gives access. Colors are + `Color`; compare against the same token constants (not literals) so intent, not pixels, is asserted. + +### Checkpoint +- [ ] `./Alfie/scripts/verify.sh` passes. +- [ ] Acceptance criteria above all met. +- [ ] Manual: run the 4 `#Preview`s in ThemedButton.swift — layout/structure unchanged. + +### Depends on +none diff --git a/Docs/Plans/ALFMOB-271-refactor-themedbutton-tokens/phase-2-sizing-literals.md b/Docs/Plans/ALFMOB-271-refactor-themedbutton-tokens/phase-2-sizing-literals.md new file mode 100644 index 00000000..2e5cc9f4 --- /dev/null +++ b/Docs/Plans/ALFMOB-271-refactor-themedbutton-tokens/phase-2-sizing-literals.md @@ -0,0 +1,26 @@ +## Phase 2: Sizing literals + gap documentation + +### Goal +Tokenize the one raw literal that has an exact token (`iconSize`), and document the literals that +genuinely have no token so the file honestly reflects token coverage. + +### Acceptance criteria +- [ ] `Constants.iconSize` references `Sizing.iconsIconSmall` (== 16, identical rendering). +- [ ] `smallHeight/mediumHeight/bigHeight` (36/44/52) and the `lineWidth: 1` stroke carry a comment + noting no design token exists (gap), so they are not mistaken for un-migrated values. +- [ ] `./Alfie/scripts/verify.sh` green. + +### Steps +1. **iconSize → token** (file: `ThemedButton.swift:139`, size: XS) — `static let iconSize: CGFloat = Sizing.iconsIconSmall`. + Why: `Sizing.iconsIconSmall` resolves to `Primitives.Spacing.spacing16` = 16 — same value, now tokenized (AC1). +2. **Document literal gaps** (file: `ThemedButton.swift:140-142,184`, size: XS) — add a comment above the + height constants and the `.stroke(..., lineWidth: 1)` call: `// no design token for button height / 1pt stroke`. + Why: makes the remaining literals intentional & reviewable, not oversights. + +### Checkpoint +- [ ] `./Alfie/scripts/verify.sh` passes. +- [ ] Acceptance criteria above all met. +- [ ] Manual: previews unchanged (icon size 16 identical). + +### Depends on +Phase 1 (same files region; keeps a single clean diff on ThemedButton.swift) diff --git a/Docs/Plans/ALFMOB-271-refactor-themedbutton-tokens/plan.md b/Docs/Plans/ALFMOB-271-refactor-themedbutton-tokens/plan.md new file mode 100644 index 00000000..91943345 --- /dev/null +++ b/Docs/Plans/ALFMOB-271-refactor-themedbutton-tokens/plan.md @@ -0,0 +1,95 @@ +--- +title: Refactor ThemedButton to use design tokens +ticket: ALFMOB-271 +status: completed +complexity: LOW +mode: auto +blockedBy: [] +blocks: [] +created: 2026-07-09 +--- + +## Overview +Move `ThemedButton` / `ButtonThemeSpec` styling onto the **semantic** generated token layer. +On this branch colors/spacing/radius already reference *primitive* generated tokens +(`Primitives.Colours.*`, `Primitives.Spacing.*`, `Sizing.radiusSoft`) — the ticket's premise of +hardcoded `Colors.primary.mono900` / `.white` is stale. The real work is (1) re-point the color +spec from raw neutrals to the semantic `Theme.button*` tokens (the single-source-of-truth the epic +intends), and (2) tokenize the last raw literals. Public `ThemedButton` API stays unchanged. + +## Acceptance Criteria (feature-level) +- [ ] AC1 ThemedButton uses generated token values for all visual properties *for which a token exists* (colors via semantic `Theme.*`; icon size via `Sizing.*`; radius/spacing already tokenized). Gaps (button heights, 1pt border, pressed/underline colors) documented in Open Questions. +- [ ] AC2 All 4 style variants render correctly (Primary, Secondary, Tertiary, Underline). +- [ ] AC3 All 3 sizes render correctly (.small/.medium/.big). +- [ ] AC4 Loading, disabled, pressed states work correctly. +- [ ] AC5 Existing call sites do not need changes (public API byte-for-byte compatible). +- [ ] AC6 Appearance validated by tests. NOTE: repo-wide snapshot infra is currently disabled + (see Open Q6); this plan validates via a token-mapping unit test and defers snapshot re-enable. + +## Approach +Adopt the semantic `Theme.button{Primary,Secondary,Terciary,Destructive}{Background,Content,Stroke}…{Default,Disabled}` +tokens in `ButtonTheme.spec`. These encode the design team's intended values and are the correct +abstraction over the raw neutrals currently in use. Consequences (deltas vs. today) are real and are +the subject of the grill: +- Secondary/Tertiary bg `neutrals0` (opaque white) → `transparentTransparent`. +- Disabled shades shift: bg `neutrals100`→`neutrals300`, text/border `neutrals400`→`neutrals500`. +- Secondary border `neutrals800`→`neutrals900`. +- No semantic token for **pressed** (all variants) → pressed stays on `Primitives.Colours.*` w/ comment. +- **Underline** has no semantic button group → its text `default`/`disabled` adopt the semantic + **link** tokens (`Theme.linkLinkPrimaryDefault`/`Disabled` — identical values today); pressed text + and its (invisible) bg/border stay on `Primitives.Colours.*`. +Tokenize `iconSize = 16` → `Sizing.iconsIconSmall` (identical value, 16). Heights `36/44/52` and the +1pt stroke have no token → remain literals, documented. + +Rejected: keeping the raw primitives untouched (would leave the button off the semantic layer the +epic is building — fails the spirit of AC1) and re-enabling the disabled snapshot suite / editing +`project.pbxproj` (out of scope, forbidden by rules_file, repo-wide concern). + +## Phases +1. **Colors → semantic token layer** — re-point `ButtonThemeSpec` values to `Theme.button*`; lock with a token-mapping unit test. +2. **Sizing literals + gap documentation** — `iconSize` → `Sizing.iconsIconSmall`; comment remaining literal gaps. + +(One vertical slice per phase; each leaves the package building & green. LOW complexity → ios-execute will run solo.) + +## File Changes (Summary Table) +| File | Module | Type | Change | Owner | +|---|---|---|---|---| +| `Alfie/AlfieKit/Sources/SharedUI/Theme/Buttons/ButtonTheme.swift` | SharedUI | edit | Map spec Default/Disabled colors to `Theme.button*`; keep pressed/underline on primitives w/ comment | - | +| `Alfie/AlfieKit/Sources/SharedUI/Theme/Buttons/ThemedButton.swift` | SharedUI | edit | `iconSize` → `Sizing.iconsIconSmall`; comment height/border-width gaps | - | +| `Alfie/AlfieKit/Tests/SharedUITests/ButtonThemeTests.swift` | SharedUITests | add | Assert `ButtonTheme..spec` maps to the expected `Theme.*` / `Primitives.*` tokens | - | + +## Feature Flag +n/a — pure styling refactor, no behavior change, not gated. + +## Testing Strategy +- **Unit (new):** `ButtonThemeTests` — for each of the 4 variants assert `spec.backgroundColor`, + `textColor`, `borderColor` (+ disabled/pressed) equal the intended token constants. This is the + runnable validation given snapshot infra is disabled; it locks the token wiring so future token + regenerations that change a mapping are caught. +- **Snapshot (deferred):** documented in Open Q6 — infra is out of target membership repo-wide; not + re-enabled here. If desired later, add `ThemedButtonSnapshotTests` in `AlfieTests/Snapshots/` + mirroring `ProductDetailsViewSnapshotTests` (4 styles × 3 sizes × normal/disabled/loading). +- **Manual:** Xcode previews already enumerate all 4 variants × states — eyeball before PR. +- `verify_command`: `./Alfie/scripts/verify.sh` green (build + unit + integration). + +## Risks & Mitigations +| Risk | Likelihood | Mitigation | +|---|---|---| +| Semantic tokens change rendered appearance (transparent bg, shade shifts) | High (by design) | Surfaced in grill/approval; deltas are neutral-shade & on a dev integration branch; previews eyeballed | +| Secondary/Tertiary transparent bg looks wrong on non-white surfaces | Low | Buttons are placed on white surfaces today; note for QA | +| Pressed/underline left on primitives looks inconsistent with "all tokens" AC | Med | Documented gap; primitives ARE generated tokens, just not semantic; no semantic token exists | +| Token-mapping unit test couples test to generated values (brittle on regen) | Low | That's the point (catch unintended remaps); test references the same `Theme.*` constants, not literals | + +## Out of Scope +- Re-enabling the repo-wide snapshot test suite / editing `project.pbxproj`. +- Adding new semantic tokens for pressed/underline (design-token JSON change — upstream). +- Any change to `ThemedButton` public API or call sites. +- Other components' token migration. + +## Decisions (grilled 2026-07-09 — see grill.md) +1. **Adopt semantic `Theme.button*` layer** for Primary/Secondary/Tertiary Default+Disabled. ✅ DECIDED. +2. **Visual deltas accepted** as design-authoritative (disabled bg `neutrals100→300`, text/border `neutrals400→500`, Secondary stroke `neutrals800→900`, Secondary/Tertiary bg `neutrals0→transparentTransparent`). ✅ DECIDED. +3. **Pressed state** stays on `Primitives.Colours.*` (no semantic token) with an explanatory comment. ✅ DECIDED (code: no pressed token exists). +4. **Underline variant** text `default→Theme.linkLinkPrimaryDefault`, `disabled→Theme.linkLinkPrimaryDisabled` (semantic link layer). Pressed text + bg/border stay on `Primitives.Colours.*` (no link pressed/bg/border token). ✅ DECIDED. +5. **Snapshot AC (AC6):** satisfied via token-mapping unit test now; snapshot-suite re-enable is **out of scope** (repo-wide, needs forbidden `project.pbxproj` edit). No follow-up ticket. ✅ DECIDED. +6. **`iconSize → Sizing.iconsIconSmall`**; button heights `36/44/52` and 1pt stroke stay literals (no token exists), documented with comments. ✅ DECIDED (code: `Sizing.iconsIconSmall==16`, no height/stroke tokens). diff --git a/Docs/Plans/ALFMOB-271-refactor-themedbutton-tokens/scope.md b/Docs/Plans/ALFMOB-271-refactor-themedbutton-tokens/scope.md new file mode 100644 index 00000000..86fc9667 --- /dev/null +++ b/Docs/Plans/ALFMOB-271-refactor-themedbutton-tokens/scope.md @@ -0,0 +1,38 @@ +# Scout Report: ThemedButton → design tokens (ALFMOB-271) + +**Branch**: ALFMOB-271-refactor-themedbutton-tokens (off feat/ALFMOB-264-adopt-design-tokens) **Agents**: 3 + +## ⚠️ Ticket premise is partly stale +The ticket says styling is hardcoded like `Colors.primary.mono900` / `.white`. **Not true on this +branch.** `Colors.primary.*` / `Colors.secondary.*` facades do not exist anywhere in the repo. Colors, +spacing and corner radius are **already tokenized**. So this is a *narrower* refactor than the ticket implies. + +## Relevant Files (implementation) +- `Alfie/AlfieKit/Sources/SharedUI/Theme/Buttons/ButtonTheme.swift:6-19` — `ButtonThemeSpec` struct: color-only fields, each in normal/disabled/pressed triplets (background / text / border). No width, radius, padding, or font in the spec. +- `ButtonTheme.swift:29-83` — per-variant spec values, all via `Primitives.Colours.neutrals*` (Primary bg `neutrals800`, text `neutrals0`, etc.). Primary/Secondary/Tertiary/Underline. +- `ThemedButton.swift:16-36` — public `init(text:type:style:leadingAsset:trailingAsset:isDisabled:isLoading:isFullWidth:action:)`. **API must stay unchanged.** +- `ThemedButton.swift:114-131,135-143` — `ButtonType` (.small/.medium/.big) + `Constants`: heights `36/44/52` (raw), `iconSize = 16` (raw), `horizontalPadding = 0`, `verticalPadding = -Primitives.Spacing.spacing8`, `cornerRadius = Sizing.radiusSoft`. +- `ThemedButton.swift:166,177,184` — applied spacing `Primitives.Spacing.spacing16`/`spacing8`; border `lineWidth: 1` (raw). +- `ThemedButton.swift:71-109` — font per size/variant via `theme.font.body.*` / `theme.font.heading.small` facade (wraps generated `Typography.*`). +- `ThemedButton.swift:283-316` — `CustomShimmerable` (public cornerRadius + shimmer colors via `Primitives.Colours.neutrals600/800`). +- `ThemedButton+Extension.swift:4-24` — convenience init (no `isFullWidth`). + +## Available tokens (SharedUI/GeneratedTokens — generated, do not edit) +- **Semantic button colors — `Theme.*`** (`Theme+Generated.swift:9-32`): `buttonPrimary{Background,Content,Stroke}Primary{Default,Disabled}`, `buttonSecondary…`, `buttonTerciary…` (sic), `buttonDestructive…`. **Only Default + Disabled — no Pressed variant.** +- **Primitive colors — `Primitives.Colours.*`**: `neutrals0..900`, `semanticError100..800`, `semanticSuccess100..800`, `transparentTransparent`. No pure black; darkest is `neutrals800/900`. No blue/orange/yellow/brand. +- **Sizing — `Sizing.*`**: `radiusSoft`(4)/`radiusStrong`(16)/`radiusRounded`(1000); `iconsIconSmall/Medium/Large/Xlarge`; `interactiveSmallPaddingLeftRight`/`…TopBottom`(8). No token for button heights 36/44/52. +- **Spacing — `Primitives.Spacing.spacingN`**; **Typography — `Typography.{Body,Display,Heading,Label,Link}.*`** via `theme.font.*`. + +## Migrated-component conventions to mirror +- `Components/Chips/Chip.swift`, `Components/Snackbar/SnackbarView.swift` — `Sizing.radius*`, `Primitives.Spacing.*`, `Primitives.Colours.*`, `Text.build(theme.font.body.small(...))`. + +## Tests +- **No existing tests** for ThemedButton/ButtonTheme (unit or snapshot). +- Snapshot harness: swift-snapshot-testing `1.18.3`, tests live in **AlfieTests/Snapshots/** (main app target). Helper `Snapshotting.defaultImage()`, `view.embededInContainer()`. One `func test_…` per permutation (no loops). +- ⚠️ **All snapshot test files are currently OUT of target membership** (`// TODO: Re-add Target Membership once Snapshot tests are checked for working properly`) and **no reference images are committed**. Snapshot testing is effectively disabled project-wide right now. + +## Open decisions (for PLAN/GRILL) +1. **Colors: keep raw `Primitives.Colours.neutrals*` or move to semantic `Theme.button*`?** Semantic is the "correct" token layer and the likely intent of the epic. +2. **Pressed state** — semantic `Theme.button*` has no Pressed token. Keep pressed on primitives, or derive? +3. **Raw literals** (`iconSize 16`, heights `36/44/52`, border `lineWidth 1`, `horizontalPadding 0`) — tokenize where a token exists (`iconSize → Sizing.iconsIcon*`?), accept the rest as gaps? +4. **Snapshot tests** — infra is disabled (no target membership, no committed refs). Add tests + re-enable membership + commit refs, or defer per repo's current state? From cea122ee409eabe7228412f54aa3d19426ce07b5 Mon Sep 17 00:00:00 2001 From: Khoi Nguyen Date: Thu, 9 Jul 2026 14:17:42 +0700 Subject: [PATCH 3/6] chore: remove plan after PR (ALFMOB-271) --- .../_status.md | 23 ----- .../grill.md | 25 ----- .../phase-1-colors-semantic.md | 54 ----------- .../phase-2-sizing-literals.md | 26 ----- .../plan.md | 95 ------------------- .../scope.md | 38 -------- 6 files changed, 261 deletions(-) delete mode 100644 Docs/Plans/ALFMOB-271-refactor-themedbutton-tokens/_status.md delete mode 100644 Docs/Plans/ALFMOB-271-refactor-themedbutton-tokens/grill.md delete mode 100644 Docs/Plans/ALFMOB-271-refactor-themedbutton-tokens/phase-1-colors-semantic.md delete mode 100644 Docs/Plans/ALFMOB-271-refactor-themedbutton-tokens/phase-2-sizing-literals.md delete mode 100644 Docs/Plans/ALFMOB-271-refactor-themedbutton-tokens/plan.md delete mode 100644 Docs/Plans/ALFMOB-271-refactor-themedbutton-tokens/scope.md diff --git a/Docs/Plans/ALFMOB-271-refactor-themedbutton-tokens/_status.md b/Docs/Plans/ALFMOB-271-refactor-themedbutton-tokens/_status.md deleted file mode 100644 index d0d3cc5d..00000000 --- a/Docs/Plans/ALFMOB-271-refactor-themedbutton-tokens/_status.md +++ /dev/null @@ -1,23 +0,0 @@ -# ALFMOB-271 — Refactor ThemedButton to use design tokens - -- **Ticket:** ALFMOB-271 (Story, Epic ALFMOB-264) — https://mindera.atlassian.net/browse/ALFMOB-271 -- **Base branch (PR target):** feat/ALFMOB-264-adopt-design-tokens -- **Work branch:** ALFMOB-271-refactor-themedbutton-tokens -- **High-rigor:** no - -## Summary -Update `ThemedButton` / `ButtonTheme` to read all styling (colors, typography, spacing, -corner radius, border) from generated design tokens instead of hardcoded references. -Keep the public `ThemedButton` API unchanged. Cover 4 styles × 3 sizes × states with snapshots. - -Files: `Alfie/AlfieKit/Sources/SharedUI/Theme/Buttons/{ThemedButton.swift,ButtonTheme.swift,ThemedButton+Extension.swift}` - -## Phase checklist -- [x] SCOUT → scope.md -- [x] PLAN → plan.md -- [x] GRILL → grill.md + hardened plan.md -- [x] APPROVAL gate ← approved 2026-07-09 -- [x] IMPLEMENT (ios-execute: verify ✅ + review APPROVED) -- [ ] COMMIT -- [ ] PR → feat/ALFMOB-264-adopt-design-tokens -- [ ] TICKET → In Review diff --git a/Docs/Plans/ALFMOB-271-refactor-themedbutton-tokens/grill.md b/Docs/Plans/ALFMOB-271-refactor-themedbutton-tokens/grill.md deleted file mode 100644 index a408cf32..00000000 --- a/Docs/Plans/ALFMOB-271-refactor-themedbutton-tokens/grill.md +++ /dev/null @@ -1,25 +0,0 @@ -# Grill: Refactor ThemedButton to use design tokens -**Plan**: Docs/Plans/ALFMOB-271-refactor-themedbutton-tokens/plan.md **Ticket**: ALFMOB-271 **Date**: 2026-07-09 **Branch**: ALFMOB-271-refactor-themedbutton-tokens - -## Decisions -| # | Decision | Recommended | Chosen | Plan change | -|---|---|---|---|---| -| 1 | Semantic `Theme.button*` layer vs keep primitives | Adopt semantic | **Adopt semantic** | Approach + phase-1 mapping table point Primary/Secondary/Tertiary Default+Disabled at `Theme.button*` | -| 2 | Accept visual deltas from semantic values | Yes | **Yes** | Recorded as design-authoritative in Approach/Risks | -| 3 | Satisfy snapshot AC6 given disabled infra | Unit test now, defer snapshots | **Unit test now, defer** (no follow-up ticket) | AC6 reworded; snapshot re-enable stays Out of Scope | -| 4 | Underline color source (no semantic button group) | Keep primitives | **Use `Theme.linkLink*`** (text default/disabled) | Approach + phase-1: underline text→link tokens; pressed/bg/border→primitives | - -## Answered by the codebase (not asked) -- **Pressed state has no semantic token** → must stay on `Primitives.Colours.*`. `Theme+Generated.swift:9-32` exposes only `…Default`/`…Disabled` per button group; no pressed. (auto-resolved) -- **`iconSize` tokenization** → `Sizing.iconsIconSmall` == `Primitives.Spacing.spacing16` == 16, exact match. `Sizing+Generated.swift:9`. (auto-resolved) -- **Button heights & 1pt stroke have no token** → `Sizing+Generated.swift` has no 36/44/52 height or stroke-width token; remain documented literals. (auto-resolved) -- **Ticket premise stale** (`Colors.primary.mono900`) → that facade doesn't exist on this branch; colors already on `Primitives.Colours.*`. scope.md + `ButtonTheme.swift:31-81`. (auto-resolved) - -## Assumptions surfaced -- "Use generated tokens" was implicitly read as "primitive tokens"; the plan makes explicit the intent is the **semantic** `Theme.*` layer, and that this is a *deliberate visual change*, not a no-op. -- Underline text default/disabled numerically equal the link tokens **today** — mapping is zero-pixel now but couples underline to the design system's link color going forward (accepted). -- Snapshot suite being out of target membership is treated as a pre-existing repo condition, not this ticket's problem to fix. - -## Still open (owner) -- Repo-wide snapshot-suite re-enable (target membership + committed refs) — **Design/Platform**, tracked outside ALFMOB-271; not blocking this PR. -- Whether the accepted disabled-shade shifts need Design sign-off — **Design/PM**; low-risk (neutral shades, dev integration branch), proceeding without a hard gate. diff --git a/Docs/Plans/ALFMOB-271-refactor-themedbutton-tokens/phase-1-colors-semantic.md b/Docs/Plans/ALFMOB-271-refactor-themedbutton-tokens/phase-1-colors-semantic.md deleted file mode 100644 index c93ed780..00000000 --- a/Docs/Plans/ALFMOB-271-refactor-themedbutton-tokens/phase-1-colors-semantic.md +++ /dev/null @@ -1,54 +0,0 @@ -## Phase 1: Colors → semantic token layer - -### Goal -`ButtonThemeSpec` sources its Default/Disabled colors from the semantic `Theme.button*` tokens -instead of raw `Primitives.Colours.neutrals*`, locked by a unit test. App/package builds & green. - -### Acceptance criteria -- [ ] Primary/Secondary/Tertiary Default + Disabled bg/text/border read from `Theme.button*` tokens. -- [ ] Underline text Default/Disabled read from `Theme.linkLinkPrimary*`; its pressed text + bg/border - stay `Primitives.Colours.*`. -- [ ] Pressed colors (all variants) remain `Primitives.Colours.*` with a one-line comment (no semantic token). -- [ ] `ButtonThemeTests` asserts each variant's spec fields equal the intended token constants. -- [ ] `./Alfie/scripts/verify.sh` green; Xcode previews render all 4 variants unchanged in structure. - -### Token mapping (from Theme+Generated.swift — DO NOT edit generated file) -| Variant | field | new token | -|---|---|---| -| primary | backgroundColor | `Theme.buttonPrimaryBackgroundPrimaryDefault` | -| primary | backgroundDisabledColor | `Theme.buttonPrimaryBackgroundPrimaryDisabled` | -| primary | textColor | `Theme.buttonPrimaryContentPrimaryDefault` | -| primary | textDisabledColor | `Theme.buttonPrimaryContentPrimaryDisabled` | -| primary | borderColor | `Theme.buttonPrimaryStrokePrimaryDefault` | -| primary | borderDisabledColor | `Theme.buttonPrimaryStrokePrimaryDisabled` | -| secondary | backgroundColor / Disabled | `Theme.buttonSecondaryBackgroundSecondaryDefault` / `…Disabled` | -| secondary | textColor / Disabled | `Theme.buttonSecondaryContentSecondaryDefault` / `…Disabled` | -| secondary | borderColor / Disabled | `Theme.buttonSecondaryStrokeSecondaryDefault` / `…Disabled` | -| tertiary | background/text/border Default+Disabled | `Theme.buttonTerciary…` (note generated spelling "Terciary") | -| underline | textColor | `Theme.linkLinkPrimaryDefault` | -| underline | textDisabledColor | `Theme.linkLinkPrimaryDisabled` | -| underline | textPressed + background/border (all states) | keep `Primitives.Colours.*` (no link pressed/bg/border token) | -| *pressed (all other variants)* | background/text/border Pressed | keep `Primitives.Colours.*` (no semantic token) | - -### Steps -1. **Re-point Primary/Secondary/Tertiary Default+Disabled colors** (file: `ButtonTheme.swift:31-68`, size: XS) — - swap the raw `Primitives.Colours.*` for the mapped `Theme.button*` constants per the table. - Leave the three `*PressedColor` fields on `Primitives.Colours.*`. Add a `// no semantic pressed token` - comment. Why: adopt the semantic layer (AC1) while preserving pressed behavior. -2. **Underline text → semantic link tokens** (file: `ButtonTheme.swift:70-81`, size: XS) — set - `textColor: Theme.linkLinkPrimaryDefault`, `textDisabledColor: Theme.linkLinkPrimaryDisabled`; - keep `textPressedColor` + all bg/border on `Primitives.Colours.*`. Add comment - `// underline: no semantic button group → link tokens for text; primitives for pressed/bg/border`. -3. **Add token-mapping unit test** (file: `Alfie/AlfieKit/Tests/SharedUITests/ButtonThemeTests.swift`, size: S) — - new `XCTestCase` (or Swift Testing `@Test`, match SharedUITests convention). For each variant assert - `ButtonTheme..spec. == `. `ButtonThemeSpec` is - `internal` and tests are in the same package → `@testable import SharedUI` gives access. Colors are - `Color`; compare against the same token constants (not literals) so intent, not pixels, is asserted. - -### Checkpoint -- [ ] `./Alfie/scripts/verify.sh` passes. -- [ ] Acceptance criteria above all met. -- [ ] Manual: run the 4 `#Preview`s in ThemedButton.swift — layout/structure unchanged. - -### Depends on -none diff --git a/Docs/Plans/ALFMOB-271-refactor-themedbutton-tokens/phase-2-sizing-literals.md b/Docs/Plans/ALFMOB-271-refactor-themedbutton-tokens/phase-2-sizing-literals.md deleted file mode 100644 index 2e5cc9f4..00000000 --- a/Docs/Plans/ALFMOB-271-refactor-themedbutton-tokens/phase-2-sizing-literals.md +++ /dev/null @@ -1,26 +0,0 @@ -## Phase 2: Sizing literals + gap documentation - -### Goal -Tokenize the one raw literal that has an exact token (`iconSize`), and document the literals that -genuinely have no token so the file honestly reflects token coverage. - -### Acceptance criteria -- [ ] `Constants.iconSize` references `Sizing.iconsIconSmall` (== 16, identical rendering). -- [ ] `smallHeight/mediumHeight/bigHeight` (36/44/52) and the `lineWidth: 1` stroke carry a comment - noting no design token exists (gap), so they are not mistaken for un-migrated values. -- [ ] `./Alfie/scripts/verify.sh` green. - -### Steps -1. **iconSize → token** (file: `ThemedButton.swift:139`, size: XS) — `static let iconSize: CGFloat = Sizing.iconsIconSmall`. - Why: `Sizing.iconsIconSmall` resolves to `Primitives.Spacing.spacing16` = 16 — same value, now tokenized (AC1). -2. **Document literal gaps** (file: `ThemedButton.swift:140-142,184`, size: XS) — add a comment above the - height constants and the `.stroke(..., lineWidth: 1)` call: `// no design token for button height / 1pt stroke`. - Why: makes the remaining literals intentional & reviewable, not oversights. - -### Checkpoint -- [ ] `./Alfie/scripts/verify.sh` passes. -- [ ] Acceptance criteria above all met. -- [ ] Manual: previews unchanged (icon size 16 identical). - -### Depends on -Phase 1 (same files region; keeps a single clean diff on ThemedButton.swift) diff --git a/Docs/Plans/ALFMOB-271-refactor-themedbutton-tokens/plan.md b/Docs/Plans/ALFMOB-271-refactor-themedbutton-tokens/plan.md deleted file mode 100644 index 91943345..00000000 --- a/Docs/Plans/ALFMOB-271-refactor-themedbutton-tokens/plan.md +++ /dev/null @@ -1,95 +0,0 @@ ---- -title: Refactor ThemedButton to use design tokens -ticket: ALFMOB-271 -status: completed -complexity: LOW -mode: auto -blockedBy: [] -blocks: [] -created: 2026-07-09 ---- - -## Overview -Move `ThemedButton` / `ButtonThemeSpec` styling onto the **semantic** generated token layer. -On this branch colors/spacing/radius already reference *primitive* generated tokens -(`Primitives.Colours.*`, `Primitives.Spacing.*`, `Sizing.radiusSoft`) — the ticket's premise of -hardcoded `Colors.primary.mono900` / `.white` is stale. The real work is (1) re-point the color -spec from raw neutrals to the semantic `Theme.button*` tokens (the single-source-of-truth the epic -intends), and (2) tokenize the last raw literals. Public `ThemedButton` API stays unchanged. - -## Acceptance Criteria (feature-level) -- [ ] AC1 ThemedButton uses generated token values for all visual properties *for which a token exists* (colors via semantic `Theme.*`; icon size via `Sizing.*`; radius/spacing already tokenized). Gaps (button heights, 1pt border, pressed/underline colors) documented in Open Questions. -- [ ] AC2 All 4 style variants render correctly (Primary, Secondary, Tertiary, Underline). -- [ ] AC3 All 3 sizes render correctly (.small/.medium/.big). -- [ ] AC4 Loading, disabled, pressed states work correctly. -- [ ] AC5 Existing call sites do not need changes (public API byte-for-byte compatible). -- [ ] AC6 Appearance validated by tests. NOTE: repo-wide snapshot infra is currently disabled - (see Open Q6); this plan validates via a token-mapping unit test and defers snapshot re-enable. - -## Approach -Adopt the semantic `Theme.button{Primary,Secondary,Terciary,Destructive}{Background,Content,Stroke}…{Default,Disabled}` -tokens in `ButtonTheme.spec`. These encode the design team's intended values and are the correct -abstraction over the raw neutrals currently in use. Consequences (deltas vs. today) are real and are -the subject of the grill: -- Secondary/Tertiary bg `neutrals0` (opaque white) → `transparentTransparent`. -- Disabled shades shift: bg `neutrals100`→`neutrals300`, text/border `neutrals400`→`neutrals500`. -- Secondary border `neutrals800`→`neutrals900`. -- No semantic token for **pressed** (all variants) → pressed stays on `Primitives.Colours.*` w/ comment. -- **Underline** has no semantic button group → its text `default`/`disabled` adopt the semantic - **link** tokens (`Theme.linkLinkPrimaryDefault`/`Disabled` — identical values today); pressed text - and its (invisible) bg/border stay on `Primitives.Colours.*`. -Tokenize `iconSize = 16` → `Sizing.iconsIconSmall` (identical value, 16). Heights `36/44/52` and the -1pt stroke have no token → remain literals, documented. - -Rejected: keeping the raw primitives untouched (would leave the button off the semantic layer the -epic is building — fails the spirit of AC1) and re-enabling the disabled snapshot suite / editing -`project.pbxproj` (out of scope, forbidden by rules_file, repo-wide concern). - -## Phases -1. **Colors → semantic token layer** — re-point `ButtonThemeSpec` values to `Theme.button*`; lock with a token-mapping unit test. -2. **Sizing literals + gap documentation** — `iconSize` → `Sizing.iconsIconSmall`; comment remaining literal gaps. - -(One vertical slice per phase; each leaves the package building & green. LOW complexity → ios-execute will run solo.) - -## File Changes (Summary Table) -| File | Module | Type | Change | Owner | -|---|---|---|---|---| -| `Alfie/AlfieKit/Sources/SharedUI/Theme/Buttons/ButtonTheme.swift` | SharedUI | edit | Map spec Default/Disabled colors to `Theme.button*`; keep pressed/underline on primitives w/ comment | - | -| `Alfie/AlfieKit/Sources/SharedUI/Theme/Buttons/ThemedButton.swift` | SharedUI | edit | `iconSize` → `Sizing.iconsIconSmall`; comment height/border-width gaps | - | -| `Alfie/AlfieKit/Tests/SharedUITests/ButtonThemeTests.swift` | SharedUITests | add | Assert `ButtonTheme..spec` maps to the expected `Theme.*` / `Primitives.*` tokens | - | - -## Feature Flag -n/a — pure styling refactor, no behavior change, not gated. - -## Testing Strategy -- **Unit (new):** `ButtonThemeTests` — for each of the 4 variants assert `spec.backgroundColor`, - `textColor`, `borderColor` (+ disabled/pressed) equal the intended token constants. This is the - runnable validation given snapshot infra is disabled; it locks the token wiring so future token - regenerations that change a mapping are caught. -- **Snapshot (deferred):** documented in Open Q6 — infra is out of target membership repo-wide; not - re-enabled here. If desired later, add `ThemedButtonSnapshotTests` in `AlfieTests/Snapshots/` - mirroring `ProductDetailsViewSnapshotTests` (4 styles × 3 sizes × normal/disabled/loading). -- **Manual:** Xcode previews already enumerate all 4 variants × states — eyeball before PR. -- `verify_command`: `./Alfie/scripts/verify.sh` green (build + unit + integration). - -## Risks & Mitigations -| Risk | Likelihood | Mitigation | -|---|---|---| -| Semantic tokens change rendered appearance (transparent bg, shade shifts) | High (by design) | Surfaced in grill/approval; deltas are neutral-shade & on a dev integration branch; previews eyeballed | -| Secondary/Tertiary transparent bg looks wrong on non-white surfaces | Low | Buttons are placed on white surfaces today; note for QA | -| Pressed/underline left on primitives looks inconsistent with "all tokens" AC | Med | Documented gap; primitives ARE generated tokens, just not semantic; no semantic token exists | -| Token-mapping unit test couples test to generated values (brittle on regen) | Low | That's the point (catch unintended remaps); test references the same `Theme.*` constants, not literals | - -## Out of Scope -- Re-enabling the repo-wide snapshot test suite / editing `project.pbxproj`. -- Adding new semantic tokens for pressed/underline (design-token JSON change — upstream). -- Any change to `ThemedButton` public API or call sites. -- Other components' token migration. - -## Decisions (grilled 2026-07-09 — see grill.md) -1. **Adopt semantic `Theme.button*` layer** for Primary/Secondary/Tertiary Default+Disabled. ✅ DECIDED. -2. **Visual deltas accepted** as design-authoritative (disabled bg `neutrals100→300`, text/border `neutrals400→500`, Secondary stroke `neutrals800→900`, Secondary/Tertiary bg `neutrals0→transparentTransparent`). ✅ DECIDED. -3. **Pressed state** stays on `Primitives.Colours.*` (no semantic token) with an explanatory comment. ✅ DECIDED (code: no pressed token exists). -4. **Underline variant** text `default→Theme.linkLinkPrimaryDefault`, `disabled→Theme.linkLinkPrimaryDisabled` (semantic link layer). Pressed text + bg/border stay on `Primitives.Colours.*` (no link pressed/bg/border token). ✅ DECIDED. -5. **Snapshot AC (AC6):** satisfied via token-mapping unit test now; snapshot-suite re-enable is **out of scope** (repo-wide, needs forbidden `project.pbxproj` edit). No follow-up ticket. ✅ DECIDED. -6. **`iconSize → Sizing.iconsIconSmall`**; button heights `36/44/52` and 1pt stroke stay literals (no token exists), documented with comments. ✅ DECIDED (code: `Sizing.iconsIconSmall==16`, no height/stroke tokens). diff --git a/Docs/Plans/ALFMOB-271-refactor-themedbutton-tokens/scope.md b/Docs/Plans/ALFMOB-271-refactor-themedbutton-tokens/scope.md deleted file mode 100644 index 86fc9667..00000000 --- a/Docs/Plans/ALFMOB-271-refactor-themedbutton-tokens/scope.md +++ /dev/null @@ -1,38 +0,0 @@ -# Scout Report: ThemedButton → design tokens (ALFMOB-271) - -**Branch**: ALFMOB-271-refactor-themedbutton-tokens (off feat/ALFMOB-264-adopt-design-tokens) **Agents**: 3 - -## ⚠️ Ticket premise is partly stale -The ticket says styling is hardcoded like `Colors.primary.mono900` / `.white`. **Not true on this -branch.** `Colors.primary.*` / `Colors.secondary.*` facades do not exist anywhere in the repo. Colors, -spacing and corner radius are **already tokenized**. So this is a *narrower* refactor than the ticket implies. - -## Relevant Files (implementation) -- `Alfie/AlfieKit/Sources/SharedUI/Theme/Buttons/ButtonTheme.swift:6-19` — `ButtonThemeSpec` struct: color-only fields, each in normal/disabled/pressed triplets (background / text / border). No width, radius, padding, or font in the spec. -- `ButtonTheme.swift:29-83` — per-variant spec values, all via `Primitives.Colours.neutrals*` (Primary bg `neutrals800`, text `neutrals0`, etc.). Primary/Secondary/Tertiary/Underline. -- `ThemedButton.swift:16-36` — public `init(text:type:style:leadingAsset:trailingAsset:isDisabled:isLoading:isFullWidth:action:)`. **API must stay unchanged.** -- `ThemedButton.swift:114-131,135-143` — `ButtonType` (.small/.medium/.big) + `Constants`: heights `36/44/52` (raw), `iconSize = 16` (raw), `horizontalPadding = 0`, `verticalPadding = -Primitives.Spacing.spacing8`, `cornerRadius = Sizing.radiusSoft`. -- `ThemedButton.swift:166,177,184` — applied spacing `Primitives.Spacing.spacing16`/`spacing8`; border `lineWidth: 1` (raw). -- `ThemedButton.swift:71-109` — font per size/variant via `theme.font.body.*` / `theme.font.heading.small` facade (wraps generated `Typography.*`). -- `ThemedButton.swift:283-316` — `CustomShimmerable` (public cornerRadius + shimmer colors via `Primitives.Colours.neutrals600/800`). -- `ThemedButton+Extension.swift:4-24` — convenience init (no `isFullWidth`). - -## Available tokens (SharedUI/GeneratedTokens — generated, do not edit) -- **Semantic button colors — `Theme.*`** (`Theme+Generated.swift:9-32`): `buttonPrimary{Background,Content,Stroke}Primary{Default,Disabled}`, `buttonSecondary…`, `buttonTerciary…` (sic), `buttonDestructive…`. **Only Default + Disabled — no Pressed variant.** -- **Primitive colors — `Primitives.Colours.*`**: `neutrals0..900`, `semanticError100..800`, `semanticSuccess100..800`, `transparentTransparent`. No pure black; darkest is `neutrals800/900`. No blue/orange/yellow/brand. -- **Sizing — `Sizing.*`**: `radiusSoft`(4)/`radiusStrong`(16)/`radiusRounded`(1000); `iconsIconSmall/Medium/Large/Xlarge`; `interactiveSmallPaddingLeftRight`/`…TopBottom`(8). No token for button heights 36/44/52. -- **Spacing — `Primitives.Spacing.spacingN`**; **Typography — `Typography.{Body,Display,Heading,Label,Link}.*`** via `theme.font.*`. - -## Migrated-component conventions to mirror -- `Components/Chips/Chip.swift`, `Components/Snackbar/SnackbarView.swift` — `Sizing.radius*`, `Primitives.Spacing.*`, `Primitives.Colours.*`, `Text.build(theme.font.body.small(...))`. - -## Tests -- **No existing tests** for ThemedButton/ButtonTheme (unit or snapshot). -- Snapshot harness: swift-snapshot-testing `1.18.3`, tests live in **AlfieTests/Snapshots/** (main app target). Helper `Snapshotting.defaultImage()`, `view.embededInContainer()`. One `func test_…` per permutation (no loops). -- ⚠️ **All snapshot test files are currently OUT of target membership** (`// TODO: Re-add Target Membership once Snapshot tests are checked for working properly`) and **no reference images are committed**. Snapshot testing is effectively disabled project-wide right now. - -## Open decisions (for PLAN/GRILL) -1. **Colors: keep raw `Primitives.Colours.neutrals*` or move to semantic `Theme.button*`?** Semantic is the "correct" token layer and the likely intent of the epic. -2. **Pressed state** — semantic `Theme.button*` has no Pressed token. Keep pressed on primitives, or derive? -3. **Raw literals** (`iconSize 16`, heights `36/44/52`, border `lineWidth 1`, `horizontalPadding 0`) — tokenize where a token exists (`iconSize → Sizing.iconsIcon*`?), accept the rest as gaps? -4. **Snapshot tests** — infra is disabled (no target membership, no committed refs). Add tests + re-enable membership + commit refs, or defer per repo's current state? From 7a2a9db5eb78d144061ce0073ff82cee6f9a9808 Mon Sep 17 00:00:00 2001 From: Khoi Nguyen Date: Thu, 9 Jul 2026 17:32:10 +0700 Subject: [PATCH 4/6] ALFMOB-271: Use Primitives.Border.borderWeightDefault for button stroke width --- .../AlfieKit/Sources/SharedUI/Theme/Buttons/ThemedButton.swift | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Alfie/AlfieKit/Sources/SharedUI/Theme/Buttons/ThemedButton.swift b/Alfie/AlfieKit/Sources/SharedUI/Theme/Buttons/ThemedButton.swift index c3e6d870..c4299946 100644 --- a/Alfie/AlfieKit/Sources/SharedUI/Theme/Buttons/ThemedButton.swift +++ b/Alfie/AlfieKit/Sources/SharedUI/Theme/Buttons/ThemedButton.swift @@ -182,8 +182,7 @@ private struct ThemedButtonStyle: ButtonStyle { .overlay( ZStack { RoundedRectangle(cornerRadius: Constants.cornerRadius) - // No design token exists for the 1pt stroke width — literal retained. - .stroke(borderColor(configuration), lineWidth: 1) + .stroke(borderColor(configuration), lineWidth: Primitives.Border.borderWeightDefault) if isLoading { LoaderView(circleDiameter: .defaultSmall, style: styleLoading) From 83f995bdaad460420c8bd5898194e11a92230f36 Mon Sep 17 00:00:00 2001 From: Khoi Nguyen Date: Fri, 10 Jul 2026 16:17:57 +0700 Subject: [PATCH 5/6] ALFMOB-271: Snap button heights to spacing tokens (36/44/52 -> 32/40/48) --- .../Sources/SharedUI/Theme/Buttons/ThemedButton.swift | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/Alfie/AlfieKit/Sources/SharedUI/Theme/Buttons/ThemedButton.swift b/Alfie/AlfieKit/Sources/SharedUI/Theme/Buttons/ThemedButton.swift index c4299946..9af1ea2f 100644 --- a/Alfie/AlfieKit/Sources/SharedUI/Theme/Buttons/ThemedButton.swift +++ b/Alfie/AlfieKit/Sources/SharedUI/Theme/Buttons/ThemedButton.swift @@ -137,10 +137,11 @@ private enum Constants { static let verticalPadding: CGFloat = -Primitives.Spacing.spacing8 static let cornerRadius: CGFloat = Sizing.radiusSoft static let iconSize: CGFloat = Sizing.iconsIconSmall - // No design token exists for button heights — literals retained. - static let smallHeight: CGFloat = 36 - static let mediumHeight: CGFloat = 44 - static let bigHeight: CGFloat = 52 + // No dedicated button-height token exists; snapped down to the nearest spacing-scale values + // (36→32, 44→40, 52→48) until the design system provides height tokens. + static let smallHeight: CGFloat = Primitives.Spacing.spacing32 + static let mediumHeight: CGFloat = Primitives.Spacing.spacing40 + static let bigHeight: CGFloat = Primitives.Spacing.spacing48 } // MARK: - ThemedButtonStyle From f26f90344e8f4c0c8647529fb81cd3bb9efaf49b Mon Sep 17 00:00:00 2001 From: Khoi Nguyen Date: Tue, 14 Jul 2026 10:26:13 +0700 Subject: [PATCH 6/6] ALFMOB-271: Add FIXME on button heights below HIG tap target (pending design) --- .../Sources/SharedUI/Theme/Buttons/ThemedButton.swift | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/Alfie/AlfieKit/Sources/SharedUI/Theme/Buttons/ThemedButton.swift b/Alfie/AlfieKit/Sources/SharedUI/Theme/Buttons/ThemedButton.swift index 9af1ea2f..47c5da3f 100644 --- a/Alfie/AlfieKit/Sources/SharedUI/Theme/Buttons/ThemedButton.swift +++ b/Alfie/AlfieKit/Sources/SharedUI/Theme/Buttons/ThemedButton.swift @@ -137,8 +137,9 @@ private enum Constants { static let verticalPadding: CGFloat = -Primitives.Spacing.spacing8 static let cornerRadius: CGFloat = Sizing.radiusSoft static let iconSize: CGFloat = Sizing.iconsIconSmall - // No dedicated button-height token exists; snapped down to the nearest spacing-scale values - // (36→32, 44→40, 52→48) until the design system provides height tokens. + // FIXME: No dedicated button-height token exists; heights are snapped to the spacing scale + // (36→32, 44→40, 52→48). This drops `.medium` (the default) to 40pt — below Apple's 44pt + // minimum tap target. Pending design confirmation on adding proper control-height tokens. static let smallHeight: CGFloat = Primitives.Spacing.spacing32 static let mediumHeight: CGFloat = Primitives.Spacing.spacing40 static let bigHeight: CGFloat = Primitives.Spacing.spacing48