Skip to content

feat(design): strength and cardio lane colour tokens - #26

Merged
DX23876 merged 1 commit into
mainfrom
redesign/a-lane-colors
Sep 17, 2026
Merged

DX23876 merged 1 commit into
mainfrom
redesign/a-lane-colors

Conversation

@DX23876

@DX23876 DX23876 commented Sep 17, 2026 •

Copy link
Copy Markdown
Owner

Summary

First package of the Training Load / Cardio / Strength redesign. Strength and Cardio both used effortColor, so the two training lanes could not be told apart. This adds one identity colour per lane to StrandPalette:

  • strengthDeep/Color/Bright/Gradient: electric teal
  • cardioDeep/Color/Bright/Gradient: violet-indigo

It covers all seven chart styles in light and dark. Signature and Health are the most vivid; Aurora and Forest are muted to fit their palettes. Orange, red and yellow are status colours and green is Charge, so a lane never reads as a status. The hex values sit in LaneColorTable, so tests read the source values directly.

No screen uses the tokens yet; the redesign packages that follow will.

Verification

  • swift test --filter LaneColorTests in Packages/StrandDesign: 4/4 pass. The tests cover:
    • the strength/cardio hue distance is at least 60° per style and mode;
    • both lanes are at least 18° from statusPositive, statusWarning, statusCritical and chargeColor;
    • strengthDeep/cardioDeep against onDarkPrimary is at least 4.5:1.
  • Full StrandDesign suite: 146 tests, 5 assertion failures, all inside testRecoveryStateWords (PlaceholderTests.swift, untouched here). It expects English recovery words and gets German ones under a German system locale.
  • python3 Tools/doc_comment_lint.py: OK.
  • The colour sheet across all styles was reviewed and approved before this PR.

Analysis migration required: no

Strength and Cardio both used effortColor, so the two training lanes were
indistinguishable. Each lane now owns one identity colour for all seven
chart styles in light and dark: electric teal for Strength and violet for
Cardio, kept clear of the status and Charge colours so a lane never reads
as a warning. The hexes live in LaneColorTable and LaneColorTests pin hue
separation and white-text contrast on the deep tones.

Analysis migration required: no
Copilot AI balanced review requested due to automatic review settings September 17, 2026 14:44

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Changes recommended

Add coverage for public accessors and shared canonical palette references.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Adds style-aware Strength and Cardio identity color tokens to StrandPalette for the upcoming redesign.

Changes:

  • Adds light/dark lane colors and gradients across seven chart styles.
  • Adds hue-separation and contrast regression tests.
  • Requires no analysis migration; tokens are not yet consumed by screens.
File summaries
File Summary
Packages/StrandDesign/Tests/StrandDesignTests/LaneColorTests.swift Tests lane color separation and contrast.
Packages/StrandDesign/Sources/StrandDesign/Palette.swift Defines lane color tables and public palette accessors.
Review details

Suppressed comments (2)

Packages/StrandDesign/Sources/StrandDesign/Palette.swift:830

  • The new tests never exercise the public token accessors introduced here: every assertion reads LaneColorTable.style(style) directly. A wiring error such as selecting the wrong field for an accessor or using the wrong gradient endpoints would therefore leave the suite green while consumers receive the wrong color. Add coverage that resolves each public strength*/cardio* token and both gradients for every style and scheme, or make the accessors consume a shared mapping that is itself exercised.
    public static var strengthDeep: Color {
        let hex = LaneColorTable.style(chartStyle).strengthDeep
        return Color(light: hex.light, dark: hex.dark)

Packages/StrandDesign/Tests/StrandDesignTests/LaneColorTests.swift:76

  • The separation and contrast checks use copied literals for the existing status, charge, effort, and onDarkPrimary tokens rather than the canonical palette definitions. If one of those tokens changes later, these tests can still pass even though a lane becomes hue-close to the new status/charge color or its real text contrast drops below 4.5:1. Keep the reference pairs in the same source tables as their accessors, or otherwise provide one shared source of truth for production and tests.
    // Existing token hex values, copied from `StrandPalette` (Palette.swift) for the hue-separation
    // checks below — NOT changed by this change; see the file for their canonical definitions.
  • Files reviewed: 2/2 changed files
  • Comments generated: 1
  • Review effort level: Lite (auto)

Note

Copilot is running an experiment and ran this review at Lite.


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines +127 to +129
private func lane(_ style: ChartStyle) -> StrandPalette.LaneColorTable.Style {
StrandPalette.LaneColorTable.style(style)
}
@DX23876
DX23876 merged commit 98a7a24 into main Sep 17, 2026
21 of 23 checks passed
DX23876 added a commit that referenced this pull request Sep 17, 2026
…o dodge an x86_64 emit-module crash

The universal macOS leg of app-build.yml crashes swift-frontend during x86_64
module emission (the -experimental-skip-non-inlinable-function-bodies path of
incremental Debug mode) with a stack dump and no source diagnostic. It failed
on every redesign PR (#26-#31) and on the upstream sync branch. Whole-module
mode, set only on the CI xcodebuild invocation, avoids that code path; local
incremental builds are unchanged.

Taken from 4bb8e9e on sync/upstream-2026-09-17, which is not merged yet.

Analysis migration required: no
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