Skip to content

Fix the brighten() channel swap and use semantic colours for placeholders - #559

Merged
bradleymackey merged 1 commit into
mainfrom
native-standardization-colour
Sep 14, 2026
Merged

bradleymackey merged 1 commit into
mainfrom
native-standardization-colour

Conversation

@bradleymackey

Copy link
Copy Markdown
Member

The bug

VaultItemColor.brighten(amount:) paired each channel with the wrong
luminance coefficient:

   red:   (red   + amount * 0.299).clamped(to: 0 ... 1),
-  green: (blue  + amount * 0.114).clamped(to: 0 ... 1),
-  blue:  (green + amount * 0.587).clamped(to: 0 ... 1),
+  green: (green + amount * 0.587).clamped(to: 0 ... 1),
+  blue:  (blue  + amount * 0.114).clamped(to: 0 ... 1),

The green output read from blue and the blue output from green, so
"brightening" rotated the hue instead of lightening it — and the two
weights were attached to the wrong channels on top of that. This feeds
readableForegroundColor(), so tag pill text has been rendered in a
hue-rotated colour for every light and dark tag.

brighten is a pure function used only for display, so this changes no
stored value. Covered by new VaultItemColorTests: per-channel weights,
a regression test for the swap, monotonicity, and unit-range clamping.

Semantic colours

  • HOTPCodePreviewView's Color.blue timer fill → .accentColor, so it
    follows the app accent rather than pinning to blue.
  • The Color.gray / Color.gray.opacity(0.3) placeholder tracks in
    TOTPCodePreviewView, HOTPCodePreviewView, OTPWidgetSmallView and
    OTPWidgetAccessoryRectangularView → Color(.quaternarySystemFill),
    the ramp meant for control fills.
  • VaultWidgets/Assets.xcassets/AccentColor.colorset was an empty stub
    ({"idiom":"universal"} with no colour) even though
    OTPWidgetSmallView does .tint(.accentColor) against it. It now
    references systemBlueColor, matching the app's own accent colorset.
  • WidgetBackground.colorset was an empty stub with no references
    anywhere in the codebase; OTPWidget already uses the native
    .containerBackground(.fill.tertiary, for: .widget). Deleted.

Not done, deliberately

VaultItemColor's gray/black/white/tagDefault constants are
left as they are
, despite gray being a flat (0.5, 0.5, 0.5) that
reads muddy in both appearances.

VaultItemColor is not a styling type. It is a persisted RGB model:
encoded into SwiftData via PersistedVaultTagEncoder, into the backup
format via VaultBackupTagDecoder, and it conforms to Digestable, so
its components feed the HMAC digests. Swapping the constants for
UIColor.systemGray and friends would change persisted and derived
values — a storage change wearing a styling change's clothes, and one
that touches the digest path. It needs its own PR and a migration story,
not a colour sweep.

For the same reason the type cannot be made appearance-adaptive at all:
it stores three Doubles, so there is no dynamic colour to resolve.

VaultItemTag+DisplayColors' brightness ladder is also kept. The
audit flagged it for using the NTSC formula rather than WCAG relative
luminance, and for Color+Constrast resolving UIColor(self).getRed(...)
against the default trait collection. That second point does not bite
here: a tag's colour is a static RGB value, so there is no
appearance-dependent resolution to get wrong. With the feed's interactive
pills already moved to .tint in the previous commit, the ladder now only
styles read-only display chips, where a hand-drawn capsule is a
reasonable thing to be.

Verification

Local, iPhone 18 Pro Max / iOS 27.0:

  • xcodebuild build-for-testing — TEST BUILD SUCCEEDED
  • Full suite, -parallel-testing-enabled NO — TEST EXECUTE SUCCEEDED,
    2842 passed, 0 failures across 24 bundles. Up 8 from 2834 — the
    four new brighten tests, counted once per configuration.
  • 6 snapshots re-recorded (TOTPCodePreviewView, HOTPCodePreviewView)
    and visually reviewed
  • make format + make lint — clean

⚠️ Automatic CI is still disabled (#548), so this is local verification only.

🤖 Generated with Claude Code

@bradleymackey
bradleymackey force-pushed the native-standardization-progress-motion branch from 4b93bfa to 8d7b546 Compare September 14, 2026 16:51
Base automatically changed from native-standardization-progress-motion to main September 14, 2026 16:51
…ders

## The bug

`VaultItemColor.brighten(amount:)` paired each channel with the wrong
luminance coefficient:

```diff
   red:   (red   + amount * 0.299).clamped(to: 0 ... 1),
-  green: (blue  + amount * 0.114).clamped(to: 0 ... 1),
-  blue:  (green + amount * 0.587).clamped(to: 0 ... 1),
+  green: (green + amount * 0.587).clamped(to: 0 ... 1),
+  blue:  (blue  + amount * 0.114).clamped(to: 0 ... 1),
```

The green output read from `blue` and the blue output from `green`, so
"brightening" rotated the hue instead of lightening it — and the two
weights were attached to the wrong channels on top of that. This feeds
`readableForegroundColor()`, so tag pill text has been rendered in a
hue-rotated colour for every light and dark tag.

`brighten` is a pure function used only for display, so this changes no
stored value. Covered by new `VaultItemColorTests`: per-channel weights,
a regression test for the swap, monotonicity, and unit-range clamping.

## Semantic colours

- `HOTPCodePreviewView`'s `Color.blue` timer fill → `.accentColor`, so it
  follows the app accent rather than pinning to blue.
- The `Color.gray` / `Color.gray.opacity(0.3)` placeholder tracks in
  `TOTPCodePreviewView`, `HOTPCodePreviewView`, `OTPWidgetSmallView` and
  `OTPWidgetAccessoryRectangularView` → `Color(.quaternarySystemFill)`,
  the ramp meant for control fills.
- `VaultWidgets/Assets.xcassets/AccentColor.colorset` was an empty stub
  (`{"idiom":"universal"}` with no colour) even though
  `OTPWidgetSmallView` does `.tint(.accentColor)` against it. It now
  references `systemBlueColor`, matching the app's own accent colorset.
- `WidgetBackground.colorset` was an empty stub with no references
  anywhere in the codebase; `OTPWidget` already uses the native
  `.containerBackground(.fill.tertiary, for: .widget)`. Deleted.

## Not done, deliberately

**`VaultItemColor`'s `gray`/`black`/`white`/`tagDefault` constants are
left as they are**, despite `gray` being a flat `(0.5, 0.5, 0.5)` that
reads muddy in both appearances.

`VaultItemColor` is not a styling type. It is a persisted RGB model:
encoded into SwiftData via `PersistedVaultTagEncoder`, into the backup
format via `VaultBackupTagDecoder`, and it conforms to `Digestable`, so
its components feed the HMAC digests. Swapping the constants for
`UIColor.systemGray` and friends would change persisted and derived
values — a storage change wearing a styling change's clothes, and one
that touches the digest path. It needs its own PR and a migration story,
not a colour sweep.

For the same reason the type cannot be made appearance-adaptive at all:
it stores three `Double`s, so there is no dynamic colour to resolve.

**`VaultItemTag+DisplayColors`' brightness ladder is also kept.** The
audit flagged it for using the NTSC formula rather than WCAG relative
luminance, and for `Color+Constrast` resolving `UIColor(self).getRed(...)`
against the default trait collection. That second point does not bite
here: a tag's colour *is* a static RGB value, so there is no
appearance-dependent resolution to get wrong. With the feed's interactive
pills already moved to `.tint` in the previous commit, the ladder now only
styles read-only display chips, where a hand-drawn capsule is a
reasonable thing to be.

## Verification

Local, iPhone 18 Pro Max / iOS 27.0:

- `xcodebuild build-for-testing` — `TEST BUILD SUCCEEDED`
- Full suite, `-parallel-testing-enabled NO` — `TEST EXECUTE SUCCEEDED`,
  **2842 passed, 0 failures** across 24 bundles. Up 8 from 2834 — the
  four new `brighten` tests, counted once per configuration.
- 6 snapshots re-recorded (`TOTPCodePreviewView`, `HOTPCodePreviewView`)
  and visually reviewed
- `make format` + `make lint` — clean

⚠️ Automatic CI is still disabled (#548), so this is local verification only.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@bradleymackey
bradleymackey force-pushed the native-standardization-colour branch from 5bb6839 to b4c8f32 Compare September 14, 2026 16:51
@bradleymackey
bradleymackey merged commit 4c1fd90 into main Sep 14, 2026
@bradleymackey
bradleymackey deleted the native-standardization-colour branch September 14, 2026 16:51
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.

1 participant