fix: keep TextField prompts as Text instead of dashFont - #9
Open
romchornyi wants to merge 1 commit into
Open
Conversation
The dashFont conversion turned two TextField prompts into some View. A prompt must be a Text, so both call sites failed to compile: SearchBar.swift:146 Cannot convert value of type 'some View' to Text AddressFieldView.swift:126 Reverts those two to .font(Font.dash.subhead), which is the correct call here regardless — dashFont pairs a font with a line height, and a line height cannot be applied to Text in the first place. Both files are wrapped in #if canImport(UIKit), so swift build on macOS never compiled them. Verified with xcodebuild against an iOS destination.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughiOS 17+ prompts in ChangesPrompt typography
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 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 |
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
2 participants
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
masterdoes not compile for iOS. ThedashFontconversion in #8 turned twoTextFieldprompts intosome View, but a prompt must be aText:What was done?
Reverts those two call sites to
.font(Font.dash.subhead).This is the correct call there regardless of the compile error:
dashFontpairs a fontwith a line height, and a line height cannot be applied to a
Textat all — it is aViewmodifier. Added a comment at both sites so the next sweep does not convert themagain.
Why CI and I both missed it:
SearchBarandAddressFieldVieware wrapped in#if canImport(UIKit), soswift buildon macOS never compiles them. The package has noiOS build step, so nothing caught it before merge.
How Has This Been Tested?
The iOS build covers the UIKit-gated files and, under Debug, the
#if DEBUGpreviews.Worth considering as a follow-up: adding an iOS build to CI, since macOS-only
verification structurally cannot see roughly the UIKit half of this library.
Breaking Changes
None — restores what the code did before #8.
Checklist:
Summary by CodeRabbit