feat: add DashIcon enums, SwitchView and dashFont line heights - #8
Conversation
|
Warning Review limit reached
Next review available in: 35 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughDashUIKit adds a typed icon asset API, migrates existing components to typed icons and ChangesDashUIKit UI updates
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant SwitchView
participant BindingBool
User->>SwitchView: Tap switch hit area
SwitchView->>BindingBool: Toggle isOn
BindingBool-->>SwitchView: Updated isOn value
SwitchView->>SwitchView: Animate track and thumb state
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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. Comment |
Implements the switch from the iOS design system (node 1841:259) as a standalone SwiftUI view: a 64x28 track with a 39x24 pill thumb inset by 2, sliding 21pt between states with a 0.2s ease-in-out. Covers all four states. Disabled is read from the isEnabled environment value rather than a parameter, so .disabled(true) on any ancestor works: on + enabled blue on + disabled blueAlpha50 off + enabled gray300Alpha50 off + disabled black1000Alpha20 The off and on colors map exactly to the switch/track-fill-off and switch/track-fill-on design variables. The design system has no track-fill-on-disabled variable, so blueAlpha50 is a stand-in and needs confirming with the designer.
Introduces DashIcon, a namespace of String-backed enums mirroring the asset catalog folder structure — 137 icons across 12 groups. The raw value is the imageset name, so a wrong name becomes a compile error instead of a blank image at runtime. DashIconAsset gives each case .source (a DashIconSource, for components that take one) and .image (a plain Image, styled by the caller). Group prefixes are dropped from case names since the enum already namespaces them: menu-account-error reads as DashIcon.Menu.accountError. Generated from the catalog rather than hand-written, and CaseIterable so a gallery preview can enumerate a group.
Replaces all 20 raw asset-name strings in the library with DashIcon cases.
Three of them were already broken by the catalog normalization and were
failing silently:
- SystemMessageView's default icon still pointed at "warning_triangle",
renamed to system-message-warning-triangle. This is a default parameter
value in shipping code, not a preview, so every SystemMessageView built
without an explicit icon rendered a blank one.
- MenuItem's preview used "menu-receive.disabled"; the imageset is now
menu-receive-disabled.
- TransactionView passed "transaction-mining" with no bundle, so it
resolved against the host app's bundle rather than the library's.
Call sites that only needed an Image collapse from
Image(dash: .custom(name, bundle: .dashUIKit)) to DashIcon.Group.case.image.
Converts every unambiguous .font(Font.dash.X) to .dashFont(.X), so the
documented design line height is applied alongside the font rather than
leaving each Text at the system's natural leading. Also maps two preview
labels off Apple's .subheadline onto .dashFont(.subhead).
Deliberately left on .font:
- The previews in LineHeight+DashUI and DashTextStyle exist precisely to
contrast bare .font against .dashFont — converting them would erase what
they demonstrate.
- SearchBar's Font.dash.footnote.weight(.semibold): the nearest token,
footnoteMedium, is .medium, so converting would change the weight.
- DashButtonSize.fontSize returns .system(size: 16/14/13/12); 14pt has no
token, so a partial conversion would leave the sizes inconsistent.
- DashAmount, NumericKeyboardView and SwapAmountView size text dynamically
or take a Font parameter. Converting those means reshaping the API to pass
DashTextStyle, which is a separate change.
The design system has no track-fill-on-disabled variable, so the disabled on state was a stand-in. Aligns it with the disabled off state, which means a disabled switch reads the same either way and only the thumb position tells them apart.
113e9ae to
640298f
Compare
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
Sources/DashUIKit/Foundation/Icon_DashUI.swift (1)
30-43: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExpose conversions through the protocol contract.
sourceandimageare unavailable to callers holdingany DashIconAssetorT: DashIconAsset, because they exist only on theRawRepresentable<String>-constrained extension. Declare them as protocol requirements with these default implementations, or narrow the public protocol/documentation toassetNameonly.Proposed API shape
public protocol DashIconAsset { /// The imageset name in the asset catalog. var assetName: String { get } + var source: DashIconSource { get } + var image: Image { get } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Sources/DashUIKit/Foundation/Icon_DashUI.swift` around lines 30 - 43, Update the DashIconAsset protocol to declare source and image requirements, then retain their existing default implementations in the RawRepresentable<String>-constrained extension so callers using any DashIconAsset or T: DashIconAsset can access both conversions.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Sources/DashUIKit/Components/Switch/SwitchView.swift`:
- Around line 28-45: Update the public SwitchView accessibility configuration
after its existing gesture to expose toggle semantics: add a meaningful
accessibility label, report isOn as "On" or "Off", use the .isToggle trait
instead of only .isButton, and provide an accessibility action that toggles
isOn. Keep the existing visual styling and tap behavior unchanged.
---
Nitpick comments:
In `@Sources/DashUIKit/Foundation/Icon_DashUI.swift`:
- Around line 30-43: Update the DashIconAsset protocol to declare source and
image requirements, then retain their existing default implementations in the
RawRepresentable<String>-constrained extension so callers using any
DashIconAsset or T: DashIconAsset can access both conversions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 6082152e-e040-45be-875e-b763293f4144
📒 Files selected for processing (19)
Sources/DashUIKit/Button/DashButton.swiftSources/DashUIKit/Components/AddressFieldView.swiftSources/DashUIKit/Components/DashAmount.swiftSources/DashUIKit/Components/EnterAmount/DashBalanceView.swiftSources/DashUIKit/Components/EnterAmount/DashPickerView.swiftSources/DashUIKit/Components/EnterAmount/DualSwapAmountView.swiftSources/DashUIKit/Components/EnterAmount/EnterAmountView.swiftSources/DashUIKit/Components/EnterAmount/SwapAmountView.swiftSources/DashUIKit/Components/Illustrations/ErrorIllustration.swiftSources/DashUIKit/Components/Illustrations/SuccessIllustration.swiftSources/DashUIKit/Components/MenuItem.swiftSources/DashUIKit/Components/NumericKeyboardView.swiftSources/DashUIKit/Components/SearchBar.swiftSources/DashUIKit/Components/Switch/Components/ThumbView.swiftSources/DashUIKit/Components/Switch/SwitchView.swiftSources/DashUIKit/Components/SystemMessageView.swiftSources/DashUIKit/Components/Transaction/TransactionView.swiftSources/DashUIKit/Foundation/Icon_DashUI.swiftSources/DashUIKit/Foundation/LineHeight+DashUI.swift
SwitchView announced itself as a button with no value, and the tap gesture was not exposed as an accessibility action, so VoiceOver users could not tell the state or change it. Adds a localized On/Off value and an explicit accessibility action. The trait becomes .isToggle where available; that API is iOS 17 and this component must stay usable on iOS 14, so the older path keeps .isButton. No accessibility label is set. SwitchView takes no title, so it has no context to describe itself with — a generic label would also shadow the one the host sets, the same way SwiftUI's own Toggle requires the caller to supply it.
Issue being fixed or feature implemented
Every icon in the library was referenced by raw string, so a renamed asset failed
silently at runtime with a blank image instead of a build error — and three call sites
were already broken that way.
Text was styled with
.font(Font.dash.X), which applies the font but leaves eachTextat the system's natural leading instead of the documented design line height.
The switch component was a native
Togglewith a tint, which cannot match a design whosethumb is a 39×24 pill rather than a circle.
DashSwitchcarried a TODO saying exactlythat.
What was done?
DashIcon— typed asset namesFoundation/Icon_DashUI.swiftadds a namespace ofString-backed enums mirroring theasset catalog structure: 137 icons in 12 groups. The raw value is the imageset name, so a
wrong name is now a compile error.
DashIconAssetgives each case.source(aDashIconSource, for components that take one) and.image(a plainImage, styled bythe caller). Group prefixes drop out of case names since the enum already namespaces
them —
menu-account-errorreads asDashIcon.Menu.accountError.Generated from the catalog on disk rather than hand-written, and
CaseIterableso agallery preview can enumerate a group.
All 20 raw asset strings in the library were replaced. Three were already broken:
SystemMessageViewwarning_triangle, renamed during normalization. This is a default parameter in shipping code, not a preview, so everySystemMessageViewbuilt without an explicit icon rendered blank.MenuItempreviewmenu-receive.disabled; the imageset ismenu-receive-disabledTransactionViewtransaction-miningwith nobundle:, so it resolved against the host app's bundle rather than the library'sdashFont— line heights applied21 unambiguous
.font(Font.dash.X)call sites become.dashFont(.X), applying thedocumented line height alongside the font. Two preview labels move off Apple's
.subheadlineonto.dashFont(.subhead).17 sites deliberately stay on
.font:LineHeight+DashUIandDashTextStyleexist precisely to contrastbare
.fontagainst.dashFont. Converting them would erase what they demonstrate.SearchBarusesFont.dash.footnote.weight(.semibold); the nearest token,footnoteMedium, is.medium, so converting would change the weight.DashButtonSize.fontSizereturns.system(size: 16/14/13/12). 16/13/12 map ontocallout/footnote/caption1, but 14pt has no token, so a partial conversionwould leave the sizes inconsistent.
DashAmount,NumericKeyboardViewandSwapAmountViewsize text dynamically or takea
Fontparameter. Converting those means reshaping private signatures to passDashTextStyle, and inSwapAmountViewthe value is animated between states, soadding line height would affect layout. That is a separate change, not a mechanical
one.
SwitchViewImplements the switch from the iOS design system (node
1841:259): a 64×28 track with a39×24 pill thumb inset by 2, sliding 21pt with a 0.2s ease-in-out. Disabled is read from
the
isEnabledenvironment value rather than a parameter, so.disabled(true)on anyancestor works.
Track colors map onto design variables exactly —
gray300Alpha50isswitch/track-fill-off,blueisswitch/track-fill-on,black1000Alpha20isswitch/track-fill-off-disabled..coderabbit.yamlFilters
**/*.xcassets/**out of review so future icon imports do not blow past the filelimit and skip the review of the source changes.
How Has This Been Tested?
swift buildpasses at every commit.The refactors were verified by grep rather than by eye: zero remaining raw
.custom("…")names outside the generated file, and the 17 surviving
.font(sites are exactly theones listed above.
Icon_DashUI.swiftis generated from the catalog on disk, so everyraw value is guaranteed to name a real imageset.
SwitchViewships previews for all four states plus an interactive one.Not tested: the library is not yet consumed by
dashwallet-ios, so this is library-sideonly.
Breaking Changes
None.
DashIconis additive andSwitchViewis new.DashSwitchis untouched.Worth a decision rather than a break: the design system has no
track-fill-on-disabledvariable, so a disabled on switch currently uses the samecolour as a disabled off switch — only the thumb position tells them apart. Needs
confirming with the designer.
Also unresolved:
menu-connectionshas no dark artwork because it is absent from Figma'sMenu (dark)frame, andDashIcon.Menu.sendDisabledexists but nothing references it —the
MenuItempreview only shows the receive variant.Checklist:
Summary by CodeRabbit
New Features
Enhancements