Skip to content
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
98 changes: 60 additions & 38 deletions Alfie/AlfieKit/Sources/SharedUI/Components/Chips/Chip.swift
Original file line number Diff line number Diff line change
Expand Up @@ -42,33 +42,32 @@ public struct ChipConfiguration {
}

public struct Chip: View {
private enum Constants {
static let heightSmall = 36.0
static let heightLarge = 44.0
static let closeWidth = 12.0
static let closeHeight = 12.0
static let borderNormal = 1.0
static let borderSelected = 2.0
static let maxCounter: Int = 99
}

private let configuration: ChipConfiguration

public init(configuration: ChipConfiguration) {
self.configuration = configuration
}

private var style: ChipStyle {
ChipStyle(
type: configuration.type,
isSelected: configuration.isSelected,
isDisabled: configuration.isDisabled,
counter: configuration.counter
)
}

public var body: some View {
ZStack {
RoundedRectangle(cornerRadius: Sizing.radiusRounded)
.stroke(borderColor, lineWidth: borderWidth)
.background(RoundedRectangle(cornerRadius: Sizing.radiusRounded).fill(backgroundColor))
.stroke(style.borderColor, lineWidth: style.borderWidth)
.background(RoundedRectangle(cornerRadius: Sizing.radiusRounded).fill(style.backgroundColor))
HStack(spacing: Primitives.Spacing.spacing8) {
Text.build(theme.font.body.small(configuration.label))
.foregroundStyle(textColor)
if let counterLabel {
.foregroundStyle(style.textColor)
if let counterLabel = style.counterLabel {
Text.build(theme.font.body.small(counterLabel))
.foregroundStyle(textColor)
.foregroundStyle(style.textColor)
}
if configuration.showCloseButton {
Button(action: {
Expand All @@ -79,8 +78,8 @@ public struct Chip: View {
.renderingMode(.template)
.resizable()
.scaledToFit()
.frame(width: Constants.closeWidth, height: Constants.closeHeight)
.foregroundStyle(textColor)
.frame(width: style.closeWidth, height: style.closeHeight)
.foregroundStyle(style.textColor)
}
.frame(maxHeight: .infinity)
})
Expand All @@ -91,46 +90,69 @@ public struct Chip: View {
}
.fixedSize(horizontal: true, vertical: false)
.padding(.horizontal, Primitives.Spacing.spacing16)
.frame(height: chipHeight)
.frame(height: style.height)
}
}

/// Resolves a `Chip`'s visual styling from its state, sourced from design tokens.
/// Extracted from `Chip` so the state→token mapping is unit-testable without snapshots.
struct ChipStyle {
private enum Constants {
static let heightSmall = 36.0
static let heightLarge = 44.0
static let closeWidth = 12.0
static let closeHeight = 12.0
static let borderSelected = 2.0
static let maxCounter: Int = 99
}

private var borderColor: Color {
if configuration.isDisabled {
return Primitives.Colours.neutrals100
} else if configuration.isSelected {
return Primitives.Colours.neutrals800
let type: ChipConfiguration.ChipType
let isSelected: Bool
let isDisabled: Bool
let counter: Int?

var borderColor: Color {
if isDisabled {
return Theme.surfaceForegroundPrimary
} else if isSelected {
return Theme.contentContentPrimary
} else {
return Primitives.Colours.neutrals200
return Theme.borderSoft
}
}

private var textColor: Color {
if configuration.isDisabled {
return Primitives.Colours.neutrals400
var textColor: Color {
if isDisabled {
return Theme.contentContentPrimaryDisabled
} else {
// No Theme alias maps to neutrals600, so this remains a raw primitive.
return Primitives.Colours.neutrals600
}
}

private var backgroundColor: Color {
if configuration.isDisabled {
return Primitives.Colours.neutrals100
var backgroundColor: Color {
if isDisabled {
return Theme.surfaceForegroundPrimary
} else {
return Primitives.Colours.neutrals0
return Theme.surfaceBackgroundPrimary
}
}

private var borderWidth: CGFloat {
if configuration.isSelected {
var borderWidth: CGFloat {
if isSelected {
return Constants.borderSelected
} else {
return Constants.borderNormal
return CGFloat(Primitives.Border.borderWeightDefault)
}
}

private var chipHeight: CGFloat {
var closeWidth: CGFloat { Constants.closeWidth }

var closeHeight: CGFloat { Constants.closeHeight }

var height: CGFloat {
// swiftlint:disable vertical_whitespace_between_cases
switch configuration.type {
switch type {
case .small:
return Constants.heightSmall
case .large:
Expand All @@ -139,8 +161,8 @@ public struct Chip: View {
// swiftlint:enable vertical_whitespace_between_cases
}

private var counterLabel: String? {
guard let counter = configuration.counter else {
var counterLabel: String? {
guard let counter else {
return nil
}
return counter > Constants.maxCounter ? "\(Constants.maxCounter)+" : "\(counter)"
Expand Down
127 changes: 127 additions & 0 deletions Alfie/AlfieKit/Tests/SharedUITests/ChipStyleTests.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,127 @@
import SwiftUI
import XCTest
@testable import SharedUI

final class ChipStyleTests: XCTestCase {
// MARK: - Border color

func test_border_color_is_soft_when_unselected_and_enabled() {
let sut = ChipStyle(type: .small, isSelected: false, isDisabled: false, counter: nil)
XCTAssertEqual(sut.borderColor, Theme.borderSoft)
}

func test_border_color_is_content_primary_when_selected() {
let sut = ChipStyle(type: .small, isSelected: true, isDisabled: false, counter: nil)
XCTAssertEqual(sut.borderColor, Theme.contentContentPrimary)
}

func test_border_color_is_surface_foreground_when_disabled() {
let sut = ChipStyle(type: .small, isSelected: false, isDisabled: true, counter: nil)
XCTAssertEqual(sut.borderColor, Theme.surfaceForegroundPrimary)
}

func test_border_color_disabled_takes_precedence_over_selected() {
let sut = ChipStyle(type: .small, isSelected: true, isDisabled: true, counter: nil)
XCTAssertEqual(sut.borderColor, Theme.surfaceForegroundPrimary)
}

// MARK: - Text color

func test_text_color_is_neutrals600_when_enabled() {
let sut = ChipStyle(type: .small, isSelected: false, isDisabled: false, counter: nil)
XCTAssertEqual(sut.textColor, Primitives.Colours.neutrals600)
}

func test_text_color_is_content_primary_disabled_when_disabled() {
let sut = ChipStyle(type: .small, isSelected: false, isDisabled: true, counter: nil)
XCTAssertEqual(sut.textColor, Theme.contentContentPrimaryDisabled)
}

func test_text_color_is_unaffected_by_selection() {
let sut = ChipStyle(type: .small, isSelected: true, isDisabled: false, counter: nil)
XCTAssertEqual(sut.textColor, Primitives.Colours.neutrals600)
}

// MARK: - Background color

func test_background_color_is_surface_background_when_enabled() {
let sut = ChipStyle(type: .small, isSelected: false, isDisabled: false, counter: nil)
XCTAssertEqual(sut.backgroundColor, Theme.surfaceBackgroundPrimary)
}

func test_background_color_is_surface_foreground_when_disabled() {
let sut = ChipStyle(type: .small, isSelected: false, isDisabled: true, counter: nil)
XCTAssertEqual(sut.backgroundColor, Theme.surfaceForegroundPrimary)
}

func test_background_color_is_unaffected_by_selection() {
let sut = ChipStyle(type: .small, isSelected: true, isDisabled: false, counter: nil)
XCTAssertEqual(sut.backgroundColor, Theme.surfaceBackgroundPrimary)
}

// MARK: - Border width

func test_border_width_normal_uses_border_weight_default_token() {
let sut = ChipStyle(type: .small, isSelected: false, isDisabled: false, counter: nil)
XCTAssertEqual(sut.borderWidth, CGFloat(Primitives.Border.borderWeightDefault))
}

func test_border_width_is_thicker_when_selected() {
let sut = ChipStyle(type: .small, isSelected: true, isDisabled: false, counter: nil)
XCTAssertEqual(sut.borderWidth, 2.0)
}

func test_border_width_stays_thick_when_selected_and_disabled() {
let sut = ChipStyle(type: .small, isSelected: true, isDisabled: true, counter: nil)
XCTAssertEqual(sut.borderWidth, 2.0)
}

func test_border_width_is_normal_when_unselected_and_disabled() {
let sut = ChipStyle(type: .small, isSelected: false, isDisabled: true, counter: nil)
XCTAssertEqual(sut.borderWidth, CGFloat(Primitives.Border.borderWeightDefault))
}

// MARK: - Height

func test_height_is_36_for_small() {
let sut = ChipStyle(type: .small, isSelected: false, isDisabled: false, counter: nil)
XCTAssertEqual(sut.height, 36.0)
}

func test_height_is_44_for_large() {
let sut = ChipStyle(type: .large, isSelected: false, isDisabled: false, counter: nil)
XCTAssertEqual(sut.height, 44.0)
}

// MARK: - Counter label

func test_counter_label_is_nil_when_counter_is_nil() {
let sut = ChipStyle(type: .small, isSelected: false, isDisabled: false, counter: nil)
XCTAssertNil(sut.counterLabel)
}

func test_counter_label_shows_value_within_max() {
let sut = ChipStyle(type: .small, isSelected: false, isDisabled: false, counter: 1)
XCTAssertEqual(sut.counterLabel, "1")
}

func test_counter_label_shows_value_at_max() {
let sut = ChipStyle(type: .small, isSelected: false, isDisabled: false, counter: 99)
XCTAssertEqual(sut.counterLabel, "99")
}

func test_counter_label_caps_over_max() {
let sut = ChipStyle(type: .small, isSelected: false, isDisabled: false, counter: 100)
XCTAssertEqual(sut.counterLabel, "99+")
}

func test_counter_label_shows_zero() {
let sut = ChipStyle(type: .small, isSelected: false, isDisabled: false, counter: 0)
XCTAssertEqual(sut.counterLabel, "0")
}

func test_counter_label_shows_negative_value() {
let sut = ChipStyle(type: .small, isSelected: false, isDisabled: false, counter: -1)
XCTAssertEqual(sut.counterLabel, "-1")
}
}
26 changes: 26 additions & 0 deletions Docs/Plans/ALFMOB-273-refactor-chip-tokens/_status.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
# ALFMOB-273 — Refactor Chip component to use design tokens

- **Ticket:** [ALFMOB-273](https://mindera.atlassian.net/browse/ALFMOB-273) (Story, Major)
- **Base:** main
- **Branch:** ALFMOB-273-refactor-chip-tokens
- **Worktree:** .claude/worktrees/ALFMOB-273-refactor-chip-tokens
- **Target file:** Alfie/AlfieKit/Sources/SharedUI/Components/Chips/Chip.swift

## Goal
Replace hardcoded `Colors.*`, `Spacing.*`, `CornerRadius.*`, and typography in `Chip` with
design-token-generated values. Selected/unselected states must render identically; no call-site
changes. Mirrors the Badge (ALFMOB-269) and ThemedButton (ALFMOB-271) token refactors.

## Scope decision (user)
Adopt semantic `Theme.*` color aliases where they map cleanly + border-width token + unit tests.
Ticket premise stale (Chip already token-based); real work = semantic color migration + `borderNormal`→token + tests.

## Phase checklist
- [x] Scout → scope.md
- [x] Plan → plan.md
- [x] Grill → hardened plan.md + grill.md
- [x] Approval gate (approved 2026-07-10)
- [x] Implement (ios-execute: verify ✅ build+unit, review ✅ no Critical/Important)
- [x] Commit (e097ae6 impl + tests)
- [ ] PR → main
- [ ] Ticket transition → In Review
24 changes: 24 additions & 0 deletions Docs/Plans/ALFMOB-273-refactor-chip-tokens/grill.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
# Grill: Refactor Chip to use design tokens

**Plan**: Docs/Plans/ALFMOB-273-refactor-chip-tokens/plan.md **Ticket**: ALFMOB-273 **Date**: 2026-07-10 **Branch**: ALFMOB-273-refactor-chip-tokens

## Decisions
| # | Decision | Recommended | Chosen | Plan change |
|---|---|---|---|---|
| Q1 | How to make state→style logic unit-testable | Extract `ChipStyle` type (mirrors `BadgeHelper`) | Extract `ChipStyle` type | D9 resolved; phase-1 step 1 = extraction |
| Q2 | disabled `neutrals100` (border+bg): raw vs name-mismatched alias | Keep raw primitive | Use `Theme.surfaceForegroundPrimary` alias | D3 & D7 → `Theme.surfaceForegroundPrimary` |
| Q3 | selected border `neutrals800`: which Theme alias (principle = don't use Primitives directly) | `Theme.contentContentPrimary` | `Theme.contentContentPrimary` | D2 → `Theme.contentContentPrimary` (was raw) |

## Answered by the codebase (not asked)
- D1/D5/D6/D8 mappings — `Theme+Generated.swift:7,23,33` + `Primitives+Generated.swift:8` confirm each alias equals the exact primitive Chip uses today (`borderSoft`=neutrals200, `contentContentPrimaryDisabled`=neutrals400, `surfaceBackgroundPrimary`=neutrals0, `borderWeightDefault`=1) → zero visual change.
- D4 (default text neutrals600) stays raw — **no `Theme.*` alias exists at `neutrals600`** (grep of `Theme+Generated.swift` = 0 matches); routing it through the semantic layer would require a new generated token (out of scope). This is the sole remaining direct `Primitives` reference.
- D10 test framework = XCTest — `BadgeHelperTests.swift:2` uses XCTest + `@testable import SharedUI`; mirror the neighbour.
- "No call-site changes" achievable — scout: only `#Preview` + `ChipsDemoView.swift:17` construct `Chip`; public API untouched.
- Snapshots excluded — memory `project_snapshot_suite_disabled`: suite is out of target membership repo-wide.

## Assumptions surfaced
- The refactor is a pure semantic/naming change: every `Theme.*` alias is a static forwarder to the same `Primitives.Colours.*`, so appearance is provably unchanged; tests assert the state→token mapping as a regression guard.
- `ChipStyle` is `internal` (not public) — introducing it does not alter Chip's public surface.

## Still open (owner)
- None.
41 changes: 41 additions & 0 deletions Docs/Plans/ALFMOB-273-refactor-chip-tokens/phase-1-chip-tokens.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
## Phase 1: Chip token alignment + tests

### Goal
`Chip` sources every available token (semantic `Theme.*` colors where the alias fits; border-normal
width from `Primitives.Border.borderWeightDefault`) with zero visual change and its state→style logic
covered by unit tests.

### Acceptance criteria
- [ ] `ChipStyle` value type computes borderColor/textColor/backgroundColor/borderWidth/chipHeight/counterLabel from `(type, isSelected, isDisabled, counter)`.
- [ ] Color mapping matches the D1–D8 table in `plan.md` (D1/D2/D3/D5/D6/D7 via `Theme.*`; only D4 raw).
- [ ] `borderNormal` width comes from `Primitives.Border.borderWeightDefault` (D8); `borderSelected` stays `2.0`.
- [ ] `Chip.body` renders identically to current `main` for all state combos (verified by Preview + demo).
- [ ] Public API (`Chip(configuration:)`, `ChipConfiguration`, `ChipType`) byte-for-byte unchanged.
- [ ] `ChipStyleTests` covers every state→value mapping + counter-label boundaries; `verify.sh` green.

### Steps
1. **Extract `ChipStyle`** (file: `Chip.swift:45-147`, size: S) — move `Constants`, `borderColor`,
`textColor`, `backgroundColor`, `borderWidth`, `chipHeight`, `counterLabel` into an `internal
struct ChipStyle` initialised from `(type, isSelected, isDisabled, counter)`. Keep values
identical for now (pure move). Why: creates a `@testable` seam mirroring `BadgeHelper`.
2. **Apply token mappings** (file: `Chip.swift`, size: XS) — in `ChipStyle`: D1 unselected border →
`Theme.borderSoft`; D2 selected border → `Theme.contentContentPrimary`; D3 disabled border →
`Theme.surfaceForegroundPrimary`; D5 disabled text → `Theme.contentContentPrimaryDisabled`; D6 default
bg → `Theme.surfaceBackgroundPrimary`; D7 disabled bg → `Theme.surfaceForegroundPrimary`; D8 normal
border width → `CGFloat(Primitives.Border.borderWeightDefault)`. Only D4 (default text) stays raw
`Primitives.Colours.neutrals600` — no `Theme` alias exists at that value. Why: user-approved semantic
adoption, no visual change.
3. **Rewire `Chip.body`** (file: `Chip.swift`, size: XS) — build one `ChipStyle` from
`configuration` and read its properties; delete the now-migrated private computed props. Why:
single source of styling truth; keeps `body` thin.
4. **Add `ChipStyleTests`** (file: `Alfie/AlfieKit/Tests/SharedUITests/ChipStyleTests.swift`, size: XS) —
XCTest + `@testable import SharedUI`; assert each state→token value and `counterLabel` boundaries
(nil→nil, 1→"1", 99→"99", 100→"99+"). Why: the coverage AC; regression guard on the mapping.

### Checkpoint
- [x] `./Alfie/scripts/verify.sh --skip-integration` passes (build + unit; integration skipped — SharedUI-only change, no BFF surface).
- [x] All acceptance criteria above met (21 `ChipStyleTests` green).
- [ ] Manual: `#Preview` and `ChipsDemoView` render identically to `main` (refactor is provably no-op — every alias forwards to the same primitive).

### Depends on
none
Loading