Repository navigation
fix(search-bar): style the typed text and drive Cancel from focus - #31
Conversation
The field styled only its prompt. Typed text fell back to the system body font in `.primary`, so the line grew from 15pt to 17pt on the first keystroke and did not use the design-system colour. It now uses `.dashFont(.subhead)` and `Color.dash.primaryText`, the same modifiers as `AddressFieldView`. The Cancel button followed a `@State` copy of the focus state, synced in `onAppear` and `onChange`, and each focus change was animated three times. It now reads `isFocused` directly, with one `.animation(value:)`. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Focus behavior and text presentation Sources/DashUIKit/Components/SearchBar.swift, docs/buttons-and-inputs.md |
Cancel visibility and animation now use isFocused instead of a separate isEditing state. The cancel action clears text and resets focus. Typed text uses the Dash subhead font and primary text color. Previews show empty and typed fields in light and dark color schemes. Documentation describes the internal focus state and notes that the font does not scale with Dynamic Type. |
Estimated code review effort: 2 (Simple) | ~10 minutes
Change: Bug fix · Severity of issue fixed: Low
Possibly related PRs
- dashpay/DashUIKit#15: It changes the SearchBar implementation that this PR further updates.
Suggested reviewers: jeanpierreroma
Merge Risk: 
298c3
-
The search text now matches the documented fixed-size typography, and Cancel follows focus directly. No merge-blocking issue remains established.
Pre-merge checks: 
-
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title clearly and concisely describes the two main changes: typed-text styling and deriving Cancel visibility from focus. Linked Issues check Issue [ #26] requires design-system styling for typed text and removal of the mirroredisEditingstate.SearchBar.swiftapplies.dashFont(.subhead),Color.dash.primaryText, and the primary-text…Out of Scope Changes check The changes stay within issue [ #26]. The added previews support the requested visual states. The documentation records the changed focus and text styling behavior. No unrelated source or public API ch…Docstring Coverage No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat… Full details: Linked Issues check
-
Explanation
Issue [
#26] requires design-system styling for typed text and removal of the mirroredisEditingstate.SearchBar.swiftapplies.dashFont(.subhead),Color.dash.primaryText, and the primary-text tint. It drives Cancel visibility and the single animation fromisFocused. The Cancel action clears text and focus. The documentation records the focus behavior and fixed text style. No additional automated test requirement appears in the issue.
Full details: Out of Scope Changes check
-
Explanation
The changes stay within issue [
#26]. The added previews support the requested visual states. The documentation records the changed focus and text styling behavior. No unrelated source or public API changes appear in the reviewed diff.
Full details: Docstring Coverage
-
Explanation
No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 0 functions across 1 files. (1 skipped: 1 unsupported.)
-
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.
romchornyi-pitcrew
left a comment
romchornyi-pitcrew
left a comment
There was a problem hiding this comment.
✅ no blockers · no majors · 6 non-blocking recommendations — reviewed at e5a69b19d4
| Level | Verdict | |
|---|---|---|
| 1 · Idea | ✅ valid | #26 is real: typed text fell back to system body 17pt in .primary while the prompt is subhead 15pt; isEditing only mirrored @FocusState |
| 2 · Architecture | ✅ right layer | The defect and the fix both live in the DashUIKit component; callers in dashwallet-ios set no font or colour on SearchBar |
| 3 · Completeness | ✅ complete | Only SearchBar and AddressFieldView hold a TextField at this head, and AddressFieldView already uses this styling; both points of #26 are covered |
| 4 · Code | ✅ clean | Cancel is driven by focus with one implicit animation on the same subtree as before; typing does not trigger it |
6 non-blocking recommendations
- Placeholder colour has no dark variant —
—
Black1000Alpha30defines only auniversalcolour (near-black, 30% alpha), so in dark mode the empty-field placeholder almost vanishes, while typed text is now the adaptiveprimaryText. The PR leaves the placeholder as it was, and uses the same token, but the new "Typed, dark" preview never shows the empty dark state. An issue for an adaptive placeholder token, shared by both fields, would track it. (verified against the colorset) - Shared input-field style — — the prompt,
DashUIKit/Sources/DashUIKit/Components/SearchBar.swift
Lines 110 to 123 in e5a69b1
.dashFont(.subhead),foregroundStyleandtintchain, along with its comment, is copied from. A small shared modifier would keep the two fields from drifting apart, for example when item 1 is fixed.DashUIKit/Sources/DashUIKit/Components/AddressFieldView.swift
Lines 216 to 230 in e5a69b1
.dashFonton a single-line field — — on one linedashLineHeightadds onlydelta / 2≈ 1pt of vertical padding outside the text field, andlineSpacingdoes nothing..font(Font.dash.subhead)alone would match the prompt exactly.AddressFieldViewneeds.dashFontbecause it can wrap to two lines. Harmless either way.- Previews for the states this PR changes — — no preview shows Cancel visible or the empty field in dark mode. The repo's CLAUDE.md asks for a preview of every state. The focused state cannot be forced through the private
DashUIKit/Sources/DashUIKit/Components/SearchBar.swift
Lines 131 to 149 in e5a69b1
@FocusState, so a note in the docs that it is checked on a simulator would do. - No test for the Cancel logic — and
DashUIKit/Sources/DashUIKit/Components/SearchBar.swift
Lines 60 to 63 in e5a69b1
— the tests box is unchecked andDashUIKit/Sources/DashUIKit/Components/SearchBar.swift
Lines 95 to 98 in e5a69b1
Tests/has nothing forSearchBar. Nothing would fail if a later change brought back a lagging state copy, or cleared the text without dropping focus. - Clear-button removal on Cancel — —
DashUIKit/Sources/DashUIKit/Components/SearchBar.swift
Lines 96 to 97 in e5a69b1
text = ""andisFocused = falseare separate writes. If SwiftUI applies the focus write in a later update, the clear button disappears without animation while Cancel animates out. Worth a look on a simulator. (not verified, runtime behaviour)
The fixed 15pt size for typed text (no Dynamic Type) is a deliberate trade-off. The PR description and the docs both state it, and it matches every Font.dash token, so it is not listed as a finding.
Review details
Tier: normal (UI component logic + styling). Levels 1–3 inline. Level 4: general lane (/code-review high) and the UI specialist lane (states, animation, Dynamic Type, VoiceOver, dark mode, tokens, docs), with the class boundary set to every text-input component in DashUIKit. The verifier checked the dark-mode placeholder claim against Black1000Alpha30.colorset and the padding claim against dashLineHeight. The Cancel-animation point stays unverified. CI was green at this head.
🤖 Reviewed with Claude Code
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… an empty dark preview Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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 298c31dd3e
| Level | Verdict | |
|---|---|---|
| 1 · Idea | ✅ valid | Unchanged since the last review |
| 2 · Architecture | ✅ right layer | Unchanged |
| 3 · Completeness | ✅ complete | Unchanged |
| 4 · Code | ✅ clean | The PR's own new commits (421e134, 298c31d) move .focused after the style modifiers, which changes no behaviour, and add a preview and one doc line. The rest of the range is the merge of master. CI is green at this head |
Closed since the last review (bot review on e5a69b1)
- Previews for the states this PR changes: an "Empty, dark" preview is added, which shows the placeholder on the dark background. The focused state cannot be forced through the private
@FocusState, and the docs now say so ("Focus is internal").DashUIKit/Sources/DashUIKit/Components/SearchBar.swift
Lines 137 to 143 in 298c31d
- The field's modifiers are ordered like
AddressFieldView's, so the two read the same.DashUIKit/Sources/DashUIKit/Components/SearchBar.swift
Lines 119 to 123 in 298c31d
3 non-blocking recommendations (open from the last review)
- The placeholder colour has no dark variant —
—
Black1000Alpha30is stilluniversal-only. The new "Empty, dark" preview now shows the near-invisible placeholder that the last review described.AddressFieldViewuses the same token. No issue tracks it yet. One issue for an adaptive placeholder token, shared by both fields, would keep it from getting lost. - A shared input-field style — the two fields now match in order, but they are still two copies. A small shared modifier would keep them from drifting, for example when item 1 is fixed.
- No test for the Cancel logic — still no test in
Tests/for showing and hiding Cancel or for the Cancel action. Now that #40 adds snapshot tests, aSearchBarempty / typed reference in light and dark would pin the visual half at least.
Review details
Re-review of the bot review on e5a69b1. The range has ten commits, but eight of them are master (#30, #33–#37, #39) merged in. The bot reviewed each of those on its own PR. The PR's own diff against master is still SearchBar.swift and docs/buttons-and-inputs.md. Following the re-review rule, only level 4 was re-checked, inline, against 421e134 and 298c31d. No lane was re-run for a modifier reorder, a preview and a doc line. Dropped from the last list: the .dashFont padding nit, and the clear-button animation note, which stays unverified and needs no change.
🤖 Reviewed with Claude Code
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Issue being fixed or feature implemented
Closes #26.
SearchBarstyled only its placeholder. The text you type fell back to the system body font (17pt) in.primary, while the placeholder is.subhead(15pt). So the line changed size on the first keystroke and did not use the design-system colour. dashwallet-ios uses this field inLocalCurrencyViewandSelectCoinView.What was done?
.dashFont(.subhead),Color.dash.primaryTextand theprimaryTexttint, then.focused: the same modifiers, in the same order, asAddressFieldView, so the two fields render typed text the same way..dashFontadds about 1pt of line-height padding, which fits inside the 40pt field; the text stays vertically centred.@FocusState isFocuseddirectly. The@State isEditingcopy is gone, along with theonAppearsync, theonChangeand bothwithAnimationblocks..animation(_:value: isFocused)now covers what was animated three times.onAppearsync never had any effect:@FocusStateis alwaysfalseon first appearance, and the host cannot focus this field from outside.docs/buttons-and-inputs.mddescribes the focus-driven Cancel button, the typed-text style, and that focus is internal to the component.How Has This Been Tested?
swift build(macOS), the iOS Release build for devices and the unit tests on an iOS 18.6 simulator are clean, with no warnings.masterand this branch side by side, in light and dark, in four states: empty, focused, typed, typed + focused. Onmasterthe typed text is visibly larger than the placeholder; on this branch they match. The field stays vertically centred, Cancel appears only while focused, and dark mode usesprimaryText.develop(LocalCurrencyView,SelectCoinView) sets no font or colour onSearchBar, so nothing there is overridden.Before / After
iOS 18.6 simulator, rendered by a test harness. "Focused" shows the Cancel button.
Breaking Changes
None. The public API is unchanged.
One behaviour to know: typed text is now a fixed 15pt, like every
Font.dashtoken, the placeholder andAddressFieldView. Before, it was the system body style, which scaled with Dynamic Type. The docs say so.Checklist:
🤖 Generated with Claude Code
Summary by CodeRabbit