Repository navigation
feat(button)!: add the tinted red style - #46
Merged
Merged
Conversation
A destructive action that is not the primary one (DashConnect's Disconnect) is drawn tinted red; without the style the app had to lay a background behind a plain red button, which left only its title tappable. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 30 minutes. View limit details
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. |
romchornyi
marked this pull request as ready for review
October 31, 2026 21:00
romchornyi-pitcrew
approved these changes
Oct 31, 2026
romchornyi-pitcrew
left a comment
romchornyi-pitcrew
left a comment
There was a problem hiding this comment.
✅ no blockers · no majors · 3 non-blocking recommendations — reviewed at a25cf7aa4c
| Level | Verdict | |
|---|---|---|
| 1 · Idea | ✅ valid | Figma's btn-l tinted-red had no style, so dashwallet-ios#1277 laid a background behind .plainRed, and only the title was tappable |
| 2 · Architecture | ✅ right layer | A style plus four tokens, following .tintedBlue, backed by colour sets |
| 3 · Completeness | ✅ complete | Both backgroundColor and foregroundColor switches, the preview names, a preview, docs, README and foundation.md all list the new style |
| 4 · Code | ✅ clean | Enabled values match the Figma tokens. The disabled pair is the same neutral as .tintedBlue's (#0A0B0D at 0.05 / 0.40, the same in dark), checked against those colour sets. The test pins all four values in light and dark |
Most checks were still running when this review was written.
3 non-blocking recommendations
- A new case in a public enum — —
DashUIKit/Sources/DashUIKit/Button/DashButton.swift
Lines 69 to 70 in a25cf7a
DashButtonStyleis a public, non-frozen enum, so a host that switches over it exhaustively stops compiling at the next tag. dashwallet-ios has none: itsButton.swiftdefines its ownstruct DashButtonStyle: ButtonStyleand does not switch over DashUIKit's enum. Still, a line in the release notes helps other hosts, and thePublic APIcheck should list the case as additive. (dashwallet-iosdevelopverified) - Two reds — the content is
#EA3943, the Figma value, while the existing red tokens are#EB3842(the description says so). Two near-identical reds can drift apart. If Figma has moved to#EA3943for every red, one issue to align the others would settle which is right. - Dark mode — the enabled background is 5% red on a dark page, the same as light, which follows
.tintedBlue. A dark render of the DashConnect sheet would confirm the button still reads as a button there; the "Dark" screenshot here shows the preview grid only.
Review details
Tier: trivial (one style, four tokens and a test, following an existing pattern). Levels 1–3 one line each. Level 4 was checked inline: the diff, the colour sets against .tintedBlue's, the test, and dashwallet-ios for exhaustive switches over DashButtonStyle. No lane was run for a pattern-copy of this size.
🤖 Reviewed with Claude Code
romchornyi
changed the title
Merged
5 tasks done
Open
3 of 5 tasks
4 of 5 tasks
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.
Issue being fixed or feature implemented
The DashConnect connection sheet's Disconnect button is
btn-l tinted-redin Figma (background--button/tinted-red/bgrgba(234,57,67,0.05), content--button/tinted-red/content#EA3943).DashButtonStylehad tinted blue, gray and white but no red, so dashwallet-ios laid a background behind a.plainRedbutton, and its clear background left only the title tappable (dashpay/dashwallet-ios#1277).What was done?
DashButtonStyle.tintedRed, followingtintedBlue: four tokens (buttonTintedRedBackground,…BackgroundDisabled,buttonTintedRedContent,…ContentDisabled) backed by colour sets with light and dark entries.DashButtonStyleTests: the style returns the four tokens, and each colour set resolves under light and dark to the expected values with a non-zero alpha, so the whole background takes taps.How Has This Been Tested?
swift build(macOS);xcodebuild -destination 'generic/platform=iOS' -configuration Release CODE_SIGNING_ALLOWED=NO build; the CI test command on the iOS 27 simulator: 20 tests, 0 failures;conventions.shOK.Before / After
Breaking Changes
DashButtonStylegains the casetintedRed. The enum is public and not frozen, so a host that switches over it without adefaultstops compiling until it handles the new case. dashwallet-ios has no such switch (its own button styles are a separate type), so it builds unchanged.🤖 Generated with Claude Code