You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
feat(address-field): match the design system's label, placeholder and scan icon - #61
AddressFieldView did not match the design system: the label was footnote regular in gray500, the placeholder was lighter than the design, and the scan button showed a QR-grid glyph where the design has the corner-brackets scan glyph. Dash Wallet iOS's new Send flow (dashpay/dashwallet-ios#1358) uses this field on its Address screen.
What was done?
Label: footnote medium in Color.dash.primaryText (was footnote regular in gray500).
Placeholder: a new color token Color.dash.textFieldPlaceholder (TextFieldPlaceholder, light and dark variants from the design system; was black1000Alpha30, which has no dark variant).
Scan button: a new template icon DashIcon.Other.textFieldScanQR (text-field-scan-qr, the design's ScanQR corner-brackets glyph as a square vector SVG) tinted with the existing Color.dash.textFieldCryptoAddressIcon; its box grows from 17 to 19 pt so the 2 pt stroke is not clipped. textFieldQR stays so the public API does not break.
Disabled field: label and entered text step down to secondaryText, the placeholder to tertiaryText, as MenuItem does, so a disabled field does not read as one that takes input. New "Disabled" preview.
Every screen that uses AddressFieldView gets the new look.
How Has This Been Tested?
swift build, swift test (61 tests), .github/scripts/conventions.sh: pass.
Snapshot tests: the 10 AddressFieldViewSnapshotTests references change (5 states × light/dark); re-rendered by CI and committed with fetch-snapshots.sh.
Before / After
Before
After
Light
Dark
No visual change (refactor, tests, docs). Say why in "What was done?".
Address fields now use updated QR scan icon styling and sizing.
Disabled address fields display labels, entered text, and placeholders with state-appropriate colors; the placeholder styling is also reflected in the text prompt.
Documentation
Clarified the appearance and behavior of disabled address fields, and updated the documented color and text-field icon options.
… scan icon
- Label: footnote medium in primary text (was footnote regular in gray500).
- Placeholder: subhead in primary text at 50% opacity (was black1000Alpha30).
- Scan button: the corner-brackets scan glyph, a new template icon
DashIcon.Other.textFieldScanQR rendered in primary text; textFieldQR stays
for API compatibility.
Snapshot references are re-recorded by CI.
No actionable comments were generated in the recent review.
Walkthrough
AddressFieldView now uses disabled-aware text and placeholder colors and a revised scan icon presentation. The change adds public color and icon tokens, a disabled-state preview, and related documentation updates.
Changes: Address field presentation
Layer / File(s)
Summary
Color and icon tokens Sources/DashUIKit/Foundation/Color+DashUI.swift, Sources/DashUIKit/Foundation/Icon_DashUI.swift, docs/foundation.md
Adds the text-field placeholder color and scan icon tokens. The foundation documentation lists the scan icon and bottomNavBackground.
Address field styling and scan control Sources/DashUIKit/Components/AddressFieldView.swift, docs/buttons-and-inputs.md
Applies disabled-aware colors to the label, entered text, and placeholder. Increases the scan icon layout size, uses the new icon and its dedicated color, adds a disabled-state preview, and documents disabled-field presentation.
dashpay/DashUIKit#15: Both changes update the prompt-based AddressFieldView text field.
dashpay/DashUIKit#36: Both changes use DashIcon cases to route bundled icon assets.
Suggested reviewers:jeanpierreroma
Merge Risk: Minimal · up to 4b53f
This change updates the address field's label, placeholder and scan icon styling and adds disabled-state colors. No concrete merge-blocking risk was identified.
Pre-merge checks: 4
Check name
Status
Explanation
Title check
The title clearly and concisely describes the main changes to AddressFieldView: aligning the label, placeholder, and scan icon with the design system.
Linked Issues check
Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check
Check skipped because no linked issues were found for this pull request.
Description Check
Check skipped - CodeRabbit’s high-level summary is enabled.
Finishing Touches
Generate docstrings
Commit to this branch
Create a new PR
Generate unit tests (beta)
Commit to this branch
Create a new PR
Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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.
Reviewing files that changed from the base of the PR and between e8ae3e7 and 4b53fc5.
Files ignored due to path filters: 13
SnapshotTests/Tests/DashUIKitSnapshotTests/__Snapshots__/AddressFieldViewSnapshotTests/testDisabled.disabled-dark.png is excluded by !**/*.png
SnapshotTests/Tests/DashUIKitSnapshotTests/__Snapshots__/AddressFieldViewSnapshotTests/testDisabled.disabled-light.png is excluded by !**/*.png
SnapshotTests/Tests/DashUIKitSnapshotTests/__Snapshots__/AddressFieldViewSnapshotTests/testEmpty.empty-dark.png is excluded by !**/*.png
SnapshotTests/Tests/DashUIKitSnapshotTests/__Snapshots__/AddressFieldViewSnapshotTests/testEmpty.empty-light.png is excluded by !**/*.png
SnapshotTests/Tests/DashUIKitSnapshotTests/__Snapshots__/AddressFieldViewSnapshotTests/testError.error-dark.png is excluded by !**/*.png
SnapshotTests/Tests/DashUIKitSnapshotTests/__Snapshots__/AddressFieldViewSnapshotTests/testError.error-light.png is excluded by !**/*.png
SnapshotTests/Tests/DashUIKitSnapshotTests/__Snapshots__/AddressFieldViewSnapshotTests/testFilled.filled-dark.png is excluded by !**/*.png
SnapshotTests/Tests/DashUIKitSnapshotTests/__Snapshots__/AddressFieldViewSnapshotTests/testFilled.filled-light.png is excluded by !**/*.png
SnapshotTests/Tests/DashUIKitSnapshotTests/__Snapshots__/AddressFieldViewSnapshotTests/testLabelAccessory.label-accessory-dark.png is excluded by !**/*.png
SnapshotTests/Tests/DashUIKitSnapshotTests/__Snapshots__/AddressFieldViewSnapshotTests/testLabelAccessory.label-accessory-light.png is excluded by !**/*.png
Sources/DashUIKit/Resources/Media.xcassets/Colors/TextField/TextFieldPlaceholder.colorset/Contents.json is excluded by !**/*.xcassets/**
Sources/DashUIKit/Resources/Media.xcassets/Icons & Illustrations/Other/text-field-scan-qr.imageset/Contents.json is excluded by !**/*.xcassets/**
Sources/DashUIKit/Resources/Media.xcassets/Icons & Illustrations/Other/text-field-scan-qr.imageset/text-field-scan-qr.svg is excluded by !**/*.svg, !**/*.xcassets/**
Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.
The reason will be displayed to describe this comment to others. Learn more.
🟡 Changes suggested — 1 major (inline) · 5 non-blocking recommendations · no blockers — reviewed at 670b446c5e
Level
Verdict
1 · Idea
✅ valid
The field drifted from the design; the new placeholder also fixes a real bug: black1000Alpha30 has no dark variant, so the placeholder was near-invisible in dark mode (compare the Before/After dark images)
2 · Architecture
✅ right layer
A DashUIKit component change, picked up by every host
3 · Completeness
✅ gaps tracked
SearchBar has the same dark-mode placeholder bug (
— Color.dash.primaryText.opacity(0.5) breaks the repo rule "Theming is asset-driven … Add a new token there, not inline" (CLAUDE.md L54–56), and the result is not what the PR and docs say: PrimaryText is white at 0.9 alpha in dark mode, so the placeholder draws at 0.45, not 50 %. A colorset with its own light and dark values (e.g. TextFieldPlaceholder: #0A0A0D @ 0.5 light, white @ 0.5 dark) fixes both, and #54 can reuse it for SearchBar.
— Color.dash.textFieldCryptoAddressIcon exists for this field's icon and is unused (#141519 light, white @ 1.0 dark); primaryText draws the glyph at 0.9 in dark mode.
— 19.0002 × 19.0851 drawn .scaledToFit() into 19 × 19 ends up about 18.9 pt wide and off the pixel grid by a fraction; a 0 0 19 19 viewBox makes it exact, as the comment at
— nothing in the library uses textFieldQR now; @available(*, deprecated, renamed: "textFieldScanQR") keeps the API and tells a host that draws its own QR button to switch.
— .renderingMode(.template) repeats the imageset's template-rendering-intent: template; one of them is enough.
The disabled field looks the same as an enabled one now — with the label in primaryText, only the missing trailing buttons tell the disabled state apart (compare the disabled references). Worth checking against the design's disabled state. (not verified against Figma)
Review details
Levels 1–3 inline; level 4: a general code review at high effort plus the UI checklist (tokens, dark mode, assets, docs). The major was checked against the PrimaryText, SearchPlaceholder and TextFieldCryptoAddressIcon colorsets at this head and against the repo's CLAUDE.md; the Before/After references were compared by eye in light and dark. CI was still running at review time.
…immed disabled state
- Placeholder: new token TextFieldPlaceholder (#0A0A0D @ 0.5 light, white @ 0.5
dark) instead of primaryText at 50% opacity, which drew at 45% in dark mode.
- Scan glyph: the field's own token textFieldCryptoAddressIcon.
- text-field-scan-qr.svg: a square 19x19 viewBox so it sits on the pixel grid.
- The template rendering is set once, in the imageset.
- Disabled: the label and entered text use secondaryText and the placeholder
tertiaryText, as MenuItem dims a disabled row.
Fixed: the scan glyph uses textFieldCryptoAddressIcon.
Fixed: text-field-scan-qr.svg has a square 0 0 19 19 viewBox (paths unchanged, centred).
Not changing this here: marking textFieldQR@available(*, deprecated) drops the synthesized CaseIterable of DashIcon.Other, which DashIconTests iterates; a hand-written allCases is its own change.
Fixed: the template rendering is set once, in the imageset.
Fixed: a disabled field dims its label and text to secondaryText and the placeholder to tertiaryText, as MenuItem dims a disabled row (the design system's disabled variant could not be read here); a Disabled preview was added.
Snapshot references are re-rendered by CI for this push; Before/After will be refreshed.
The reason will be displayed to describe this comment to others. Learn more.
✅ Clean once the references are re-rendered — the major is closed · 1 non-blocking recommendation left · no blockers — re-reviewed at 4b53fc56c9
Level
Verdict
1 · Idea
✅ valid
unchanged
2 · Architecture
✅ right layer
unchanged
3 · Completeness
✅ gaps tracked
SearchBar stays with #54, which can now reuse textFieldPlaceholder
4 · Code
✅ clean
The placeholder is a token at 50 % in both modes
Closed since the last review
Major: inline placeholder colour → new TextFieldPlaceholder colorset, #0A0A0D @ 0.5 light and white @ 0.5 dark; docs updated.
Scan glyph token → textFieldCryptoAddressIcon.
Non-square SVG → 0 0 19 19 viewBox, the group shifted up by half the 0.085 overhang, so at most 0.04 pt of stroke is trimmed top and bottom.
Template set twice → the call-site .renderingMode(.template) is gone; the imageset keeps it.
Disabled state → label and text step down to secondaryText, the placeholder to tertiaryText, the same step MenuItem uses (MenuItem.swift L103); a Disabled preview was added.
Before merging
This commit changes how the disabled field draws, and the dark placeholder moves from 45 % to 50 %, but no reference image changed. Expect Snapshot tests (iOS Simulator) to fail on testDisabled (and possibly the dark empty and error images); bring the CI-rendered images in with .github/scripts/fetch-snapshots.sh as before.
1 non-blocking recommendation (still open)
Deprecate the old glyph — Icon_DashUI.swift L265 — textFieldQR is unused in the library; @available(*, deprecated, renamed: "textFieldScanQR") keeps the API and points hosts at the new icon.
Review details
Re-review of 670b446c5e..4b53fc56c9 against the earlier findings, with the author's reply on the major; the new colorset and the SVG transform were checked by hand, the disabled colours against MenuItem. CI was still queued at review time.
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
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.
romchornyi-pitcrew
left a comment
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Issue being fixed or feature implemented
AddressFieldViewdid not match the design system: the label was footnote regular in gray500, the placeholder was lighter than the design, and the scan button showed a QR-grid glyph where the design has the corner-brackets scan glyph. Dash Wallet iOS's new Send flow (dashpay/dashwallet-ios#1358) uses this field on its Address screen.What was done?
Color.dash.primaryText(was footnote regular ingray500).Color.dash.textFieldPlaceholder(TextFieldPlaceholder, light and dark variants from the design system; wasblack1000Alpha30, which has no dark variant).DashIcon.Other.textFieldScanQR(text-field-scan-qr, the design's ScanQR corner-brackets glyph as a square vector SVG) tinted with the existingColor.dash.textFieldCryptoAddressIcon; its box grows from 17 to 19 pt so the 2 pt stroke is not clipped.textFieldQRstays so the public API does not break.secondaryText, the placeholder totertiaryText, asMenuItemdoes, so a disabled field does not read as one that takes input. New "Disabled" preview.docs/buttons-and-inputs.md,docs/foundation.mdupdated.Every screen that uses
AddressFieldViewgets the new look.How Has This Been Tested?
swift build,swift test(61 tests),.github/scripts/conventions.sh: pass.AddressFieldViewSnapshotTestsreferences change (5 states × light/dark); re-rendered by CI and committed withfetch-snapshots.sh.Before / After
🤖 Generated with Claude Code
Summary by CodeRabbit