Fix the brighten() channel swap and use semantic colours for placeholders - #559
Merged
Merged
Conversation
bradleymackey
force-pushed
the
native-standardization-progress-motion
branch
from
September 14, 2026 16:51
4b93bfa to
8d7b546
Compare
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
force-pushed
the
native-standardization-colour
branch
from
September 14, 2026 16:51
5bb6839 to
b4c8f32
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The bug
VaultItemColor.brighten(amount:)paired each channel with the wrongluminance coefficient:
The green output read from
blueand the blue output fromgreen, 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 ahue-rotated colour for every light and dark tag.
brightenis a pure function used only for display, so this changes nostored value. Covered by new
VaultItemColorTests: per-channel weights,a regression test for the swap, monotonicity, and unit-range clamping.
Semantic colours
HOTPCodePreviewView'sColor.bluetimer fill →.accentColor, so itfollows the app accent rather than pinning to blue.
Color.gray/Color.gray.opacity(0.3)placeholder tracks inTOTPCodePreviewView,HOTPCodePreviewView,OTPWidgetSmallViewandOTPWidgetAccessoryRectangularView→Color(.quaternarySystemFill),the ramp meant for control fills.
VaultWidgets/Assets.xcassets/AccentColor.colorsetwas an empty stub(
{"idiom":"universal"}with no colour) even thoughOTPWidgetSmallViewdoes.tint(.accentColor)against it. It nowreferences
systemBlueColor, matching the app's own accent colorset.WidgetBackground.colorsetwas an empty stub with no referencesanywhere in the codebase;
OTPWidgetalready uses the native.containerBackground(.fill.tertiary, for: .widget). Deleted.Not done, deliberately
VaultItemColor'sgray/black/white/tagDefaultconstants areleft as they are, despite
graybeing a flat(0.5, 0.5, 0.5)thatreads muddy in both appearances.
VaultItemColoris not a styling type. It is a persisted RGB model:encoded into SwiftData via
PersistedVaultTagEncoder, into the backupformat via
VaultBackupTagDecoder, and it conforms toDigestable, soits components feed the HMAC digests. Swapping the constants for
UIColor.systemGrayand friends would change persisted and derivedvalues — 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. Theaudit flagged it for using the NTSC formula rather than WCAG relative
luminance, and for
Color+ConstrastresolvingUIColor(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
.tintin the previous commit, the ladder now onlystyles 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-parallel-testing-enabled NO—TEST EXECUTE SUCCEEDED,2842 passed, 0 failures across 24 bundles. Up 8 from 2834 — the
four new
brightentests, counted once per configuration.TOTPCodePreviewView,HOTPCodePreviewView)and visually reviewed
make format+make lint— clean🤖 Generated with Claude Code