feat(ui): redesign all merchant locations experience#774
Open
romchornyi wants to merge 6 commits into
Open
Conversation
📝 WalkthroughWalkthroughAdds an "All Merchant Locations" feature: model address and preview helpers, MerchantDAO geometry/pagination/count updates, ExploreDash forwarding helpers, AllMerchantLocationsViewModel, SwiftUI views/cells with debug previews, hosting controllers and sheet lifecycle changes, self-sizing sheet support, NavigationBar refactor, and Xcode project wiring. ChangesAll Merchant Locations Feature
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
DashWallet/Sources/UI/Explore Dash/Merchants & ATMs/List/AllMerchantLocations/AllMerchantLocationsViewController.swift (1)
42-44: 💤 Low valueMark the unimplemented initializer as unavailable.
SwiftLint (
unavailable_function) flags this. Adding@available(*, unavailable)documents intent and surfaces misuse at compile time.♻️ Proposed tweak
+ `@available`(*, unavailable) required init?(coder: NSCoder) { fatalError("init(coder:) has not been implemented") }🤖 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 `@DashWallet/Sources/UI/Explore` Dash/Merchants & ATMs/List/AllMerchantLocations/AllMerchantLocationsViewController.swift around lines 42 - 44, The required initializer init?(coder:) in AllMerchantLocationsViewController is currently implemented with fatalError and should be marked unavailable to satisfy SwiftLint; add the `@available`(*, unavailable) attribute to the required init?(coder: NSCoder) declaration in AllMerchantLocationsViewController so the initializer is explicitly unavailable (you can keep the fatalError body for safety), which documents intent and prevents accidental use at compile time.DashWallet/Sources/UI/Explore Dash/Merchants & ATMs/List/AllMerchantLocations/AllMerchantLocationsViewModel.swift (1)
30-30: 💤 Low valueDrop the explicit
= nilinitializer.SwiftLint (
implicit_optional_initialization) flags this. Optionals are implicitlynil.♻️ Proposed tweak
- `@Published` var selectedItem: ExplorePointOfUse? = nil + `@Published` var selectedItem: ExplorePointOfUse?🤖 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 `@DashWallet/Sources/UI/Explore` Dash/Merchants & ATMs/List/AllMerchantLocations/AllMerchantLocationsViewModel.swift at line 30, Remove the explicit nil initializer from the published optional property: change the declaration of selectedItem (the `@Published` var selectedItem: ExplorePointOfUse? = nil) to omit the "= nil" so the optional is implicitly initialized, satisfying the SwiftLint implicit_optional_initialization rule.
🤖 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 `@DashWallet/Sources/UI/Explore` Dash/Merchants &
ATMs/List/AllMerchantLocations/AllMerchantLocationsView.swift:
- Around line 105-107: The current compactMap that builds newKeys and the code
that assigns MKPointAnnotation.coordinate use only nil checks for
item.latitude/item.longitude; update both places to construct a
CLLocationCoordinate2D(latitude: lat, longitude: lon) and guard on its isValid
property before using it (e.g., in the closure that builds newKeys and when
creating/assigning MKPointAnnotation for each item), so invalid coordinates are
skipped and not included in keys or map annotations.
In `@DashWallet/Sources/UI/Explore` Dash/Merchants &
ATMs/List/AllMerchantLocations/AllMerchantLocationsViewModel.swift:
- Around line 74-80: The closure assigned to listModel.itemsDidChange currently
captures the local listModel strongly and causes a retain cycle; change the
closure to reference the view model's stored model (self.model) or capture
listModel weakly so the closure does not retain listModel. Specifically, in the
itemsDidChange closure (assigned to listModel.itemsDidChange) replace uses of
the local listModel with self.model (or guard let listModel = listModel else {
return } with a weak capture) and then update distanceTexts via
computeDistanceTexts(for:), set self.items and self.isLoading from the model,
ensuring only [weak self] (and if used, [weak listModel]) are captured to break
the cycle.
- Around line 126-134: The referenceLocation() function currently builds a
CLLocation from modelSearchCenter without validating the coordinate; update
referenceLocation() to first check the coordinate with
CLLocationCoordinate2DIsValid(center) (or an equivalent lat/lon bounds check)
before calling CLLocation(latitude:longitude:), and only return that CLLocation
when valid; otherwise fall back to DWLocationManager.shared.currentLocation (or
nil if not authorized). Target the referenceLocation() method and the
modelSearchCenter variable when making this change.
---
Nitpick comments:
In `@DashWallet/Sources/UI/Explore` Dash/Merchants &
ATMs/List/AllMerchantLocations/AllMerchantLocationsViewController.swift:
- Around line 42-44: The required initializer init?(coder:) in
AllMerchantLocationsViewController is currently implemented with fatalError and
should be marked unavailable to satisfy SwiftLint; add the `@available`(*,
unavailable) attribute to the required init?(coder: NSCoder) declaration in
AllMerchantLocationsViewController so the initializer is explicitly unavailable
(you can keep the fatalError body for safety), which documents intent and
prevents accidental use at compile time.
In `@DashWallet/Sources/UI/Explore` Dash/Merchants &
ATMs/List/AllMerchantLocations/AllMerchantLocationsViewModel.swift:
- Line 30: Remove the explicit nil initializer from the published optional
property: change the declaration of selectedItem (the `@Published` var
selectedItem: ExplorePointOfUse? = nil) to omit the "= nil" so the optional is
implicitly initialized, satisfying the SwiftLint
implicit_optional_initialization rule.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6b4fc68b-e992-49d2-bf35-c708046cefba
📒 Files selected for processing (12)
DashWallet.xcodeproj/project.pbxprojDashWallet/Sources/Models/Explore Dash/Infrastructure/DAO Impl/MerchantDAO.swiftDashWallet/Sources/Models/Explore Dash/Model/Entites/ExplorePointOfUse+PreviewMock.swiftDashWallet/Sources/Models/Explore Dash/Model/Entites/ExplorePointOfUse.swiftDashWallet/Sources/Models/Explore Dash/Services/GeoRestrictionService.swiftDashWallet/Sources/UI/Explore Dash/Merchants & ATMs/List/AllMerchantLocations/AllMerchantLocationsView.swiftDashWallet/Sources/UI/Explore Dash/Merchants & ATMs/List/AllMerchantLocations/AllMerchantLocationsViewController.swiftDashWallet/Sources/UI/Explore Dash/Merchants & ATMs/List/AllMerchantLocations/AllMerchantLocationsViewModel.swiftDashWallet/Sources/UI/Explore Dash/Merchants & ATMs/List/AllMerchantLocations/Components/MerchantCellRow.swiftDashWallet/Sources/UI/Explore Dash/Merchants & ATMs/List/AllMerchantLocationsViewController.swiftDashWallet/Sources/UI/Explore Dash/Merchants & ATMs/List/Cells/MerchantItemCell.swiftDashWallet/Sources/UI/Explore Dash/Merchants & ATMs/List/Cells/PointOfUseItemCell.swift
💤 Files with no reviewable changes (1)
- DashWallet/Sources/UI/Explore Dash/Merchants & ATMs/List/AllMerchantLocationsViewController.swift
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 `@DashWallet/Sources/UI/Explore` Dash/Merchants &
ATMs/List/MerchantListViewController.swift:
- Around line 340-341: The navigation title "Where to Spend" was removed when
customNavBar is nil; in configureHierarchy (and the similar block around the
code at the 351–355 region) restore setting self.title =
NSLocalizedString("Where to Spend", comment: "") whenever customNavBar == nil
(or when you are not using the custom navigation bar) so the system navigation
path shows the title; locate the configureHierarchy method and the alternate
initialization branch and add the title assignment guarded by the customNavBar
check (reference symbols: configureHierarchy, customNavBar, title).
- Around line 390-421: installCustomNavBar currently overlays the hosted nav
(UIHostingController stored in customNavBarHost) at
view.safeAreaLayoutGuide.topAnchor without reserving vertical space for it; add
an explicit height constraint to host.view (e.g.,
host.view.heightAnchor.constraint(equalToConstant: 64) or a configurable
navHeight) and change the top constraint of the underlying content (e.g.,
contentView / tableView / map view) to pin its top to
customNavBarHost.view.bottomAnchor instead of view.safeAreaLayoutGuide.topAnchor
so the table/map is pushed below the nav bar; update NSLayoutConstraint.activate
in installCustomNavBar to include host.view.heightAnchor and then modify the
code that sets the main content’s top anchor (where contentView is constrained)
to reference customNavBarHost.view.bottomAnchor.
In `@DashWallet/Sources/UI/SwiftUI` Components/NavigationBar.swift:
- Line 166: The `@Environment` attribute is currently on the same line as the
property declaration (e.g., "`@Environment`(\\.colorScheme) private var
colorScheme"), which triggers SwiftLint; move each `@Environment` attribute onto
its own line directly above its property declaration so attributes and variables
are on separate lines for all `@Environment` properties in NavigationBar.swift
(including the colorScheme property and the other `@Environment` declaration
flagged later) to satisfy SwiftFormat/SwiftLint formatting rules.
- Around line 365-370: The NavBarBackTitlePlus preview is using the wrong
control; update the preview block named NavBarBackTitlePlus so it passes
NavigationBarElement.plus to the trailing parameter instead of
NavigationBarElement.close (locate the preview that constructs NavigationBar
with leading: NavigationBarElement.back, central:
Text("Title").font(.subheadMedium), trailing: NavigationBarElement.close.button
and replace the trailing element with NavigationBarElement.plus.button) to show
the correct "Plus" variant.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: a68e8757-9c83-403e-8385-1d0d551f25bb
📒 Files selected for processing (7)
DashWallet/Sources/UI/Explore Dash/ExploreViewController.swiftDashWallet/Sources/UI/Explore Dash/Merchants & ATMs/Details/Views/POIDetailsView.swiftDashWallet/Sources/UI/Explore Dash/Merchants & ATMs/List/AllMerchantLocations/AllMerchantLocationsView.swiftDashWallet/Sources/UI/Explore Dash/Merchants & ATMs/List/AllMerchantLocations/AllMerchantLocationsViewModel.swiftDashWallet/Sources/UI/Explore Dash/Merchants & ATMs/List/MerchantListViewController.swiftDashWallet/Sources/UI/Home/HomeViewController+Shortcuts.swiftDashWallet/Sources/UI/SwiftUI Components/NavigationBar.swift
Comment on lines
340
to
+341
| override func configureHierarchy() { | ||
| title = NSLocalizedString("Where to Spend", comment: ""); | ||
| // title = NSLocalizedString("Where to Spend", comment: ""); |
There was a problem hiding this comment.
Restore the title on the system-navigation path.
When customNavBar is nil, this controller no longer sets title anywhere in the changed code, so the standard “Where to Spend” flow can lose its navigation title.
Suggested fix
override func configureHierarchy() {
-// title = NSLocalizedString("Where to Spend", comment: "");
+ if customNavBar == nil {
+ title = NSLocalizedString("Where to Spend", comment: "")
+ }
super.configureHierarchy()
tableView.register(MerchantItemCell.self, forCellReuseIdentifier: MerchantItemCell.reuseIdentifier)
}Also applies to: 351-355
🤖 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 `@DashWallet/Sources/UI/Explore` Dash/Merchants &
ATMs/List/MerchantListViewController.swift around lines 340 - 341, The
navigation title "Where to Spend" was removed when customNavBar is nil; in
configureHierarchy (and the similar block around the code at the 351–355 region)
restore setting self.title = NSLocalizedString("Where to Spend", comment: "")
whenever customNavBar == nil (or when you are not using the custom navigation
bar) so the system navigation path shows the title; locate the
configureHierarchy method and the alternate initialization branch and add the
title assignment guarded by the customNavBar check (reference symbols:
configureHierarchy, customNavBar, title).
| private var iconName: String { | ||
| colorScheme == .dark ? "controls-back-dark-mode" : "controls-back" | ||
| } | ||
| @Environment(\.colorScheme) private var colorScheme |
There was a problem hiding this comment.
Move the @Environment attributes onto their own lines.
SwiftLint is already flagging both declarations, so this file will keep carrying lint noise until they are reformatted.
Suggested fix
- `@Environment`(\.colorScheme) private var colorScheme
+ `@Environment`(\.colorScheme)
+ private var colorScheme- `@Environment`(\.colorScheme) private var colorScheme
+ `@Environment`(\.colorScheme)
+ private var colorSchemeAs per coding guidelines, **/*.swift: Use SwiftFormat and SwiftLint for Swift code formatting and linting as configured in .swiftformat and .swiftlint.yml files.
Also applies to: 223-223
🧰 Tools
🪛 SwiftLint (0.63.3)
[Warning] 166-166: Attributes with arguments or inside always_on_line_above must be on a new line instead of the same line
(attributes)
🤖 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 `@DashWallet/Sources/UI/SwiftUI` Components/NavigationBar.swift at line 166,
The `@Environment` attribute is currently on the same line as the property
declaration (e.g., "`@Environment`(\\.colorScheme) private var colorScheme"),
which triggers SwiftLint; move each `@Environment` attribute onto its own line
directly above its property declaration so attributes and variables are on
separate lines for all `@Environment` properties in NavigationBar.swift (including
the colorScheme property and the other `@Environment` declaration flagged later)
to satisfy SwiftFormat/SwiftLint formatting rules.
Why: - consolidate the local explore redesign work into a single reviewable changeset - keep the where-to-spend and all-locations flows aligned after the sheet, navigation, and pagination updates - bump the build number to 14 for the next app build What: - add custom where-to-spend navigation bar assets and related SwiftUI navigation updates - finalize merchant details and all-locations redesign flows, including native sheet behavior and map integration - add paginated all-locations loading, real total counts, infinite scroll, and segmented-bar visibility fixes - update project build settings to 8.6.0 (14) Validation: - build was not reliably runnable in this environment because xcodebuild was blocked by CoreSimulatorService issues
Contributor
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
DashWallet/Sources/UI/Explore Dash/Merchants & ATMs/Details/POIDetailsViewController.swift (1)
218-234:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAttach
UIHostingControlleras a child controller.Line 221 adds
hostingController.viewdirectly, butaddChild(_:)/didMove(toParent:)are missing. This can break child lifecycle propagation and SwiftUI hosting behavior.Suggested fix
private func showDetailsView() { guard let detailsView = makeDetailsView() else { return } let hostingController = UIHostingController(rootView: detailsView) hostingController.view.backgroundColor = .dw_secondaryBackground() hostingController.view.translatesAutoresizingMaskIntoConstraints = false - + + addChild(hostingController) contentView.addSubview(hostingController.view) - guard let hostingView = hostingController.view else { return } - + let hostingView = hostingController.view! + NSLayoutConstraint.activate([ hostingView.topAnchor.constraint(equalTo: contentView.topAnchor), hostingView.bottomAnchor.constraint(equalTo: contentView.safeAreaLayoutGuide.bottomAnchor), hostingView.leadingAnchor.constraint(equalTo: contentView.leadingAnchor), hostingView.trailingAnchor.constraint(equalTo: contentView.trailingAnchor), ]) + hostingController.didMove(toParent: self) }🤖 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 `@DashWallet/Sources/UI/Explore` Dash/Merchants & ATMs/Details/POIDetailsViewController.swift around lines 218 - 234, The UIHostingController's view is being added directly without attaching the hostingController as a child; update showDetailsView to call addChild(hostingController) before adding hostingController.view to contentView and call hostingController.didMove(toParent: self) after activating constraints so the child lifecycle is propagated correctly (keep makeDetailsView(), hostingController.view background/translates setup, adding to contentView, and NSLayoutConstraint.activate as-is, just wrap with addChild(hostingController) and didMove(toParent: self)).
🤖 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 `@DashWallet/Sources/Models/Explore` Dash/Infrastructure/DAO
Impl/MerchantDAO.swift:
- Around line 813-820: The count filtering currently builds a CLLocation from
raw latitude/longitude returned by coordinateValue(from:) and uses
centerLocation.distance(...) without validating coordinates; update the logic in
the loop (the block that calls coordinateValue(from:), constructs CLLocation and
computes distance) to first create a CLLocationCoordinate2D from the
latitude/longitude, check validity with CLLocationCoordinate2DIsValid (or
CLLocationCoordinate2D.isValid), and only then construct CLLocation and compute
distance; if the coordinate is invalid, use your fallback chain for location
data (e.g., skip the row or try alternate fields) so invalid rows do not skew
the count in MerchantDAO's counting loop.
- Line 556: The function expandedCoordinates(for bounds: ExploreMapBounds)
currently returns a 4-element tuple which triggers SwiftLint large_tuple;
replace that tuple with a small value type: declare a lightweight struct (e.g.,
ExpandedCoordinates or CoordinatesBounds) with properties swLat, neLat, swLon,
neLon, change expandedCoordinates to return that struct instead of the tuple,
update all call sites that deconstruct or index the tuple to access the new
properties (e.g., result.swLat, result.neLon), and ensure the new type is
file-scoped or nested near MerchantDAO for minimal visibility and to satisfy
linting.
- Around line 648-650: The query currently orders only by the computed
distanceExpression (distanceSorting) and then applies limit/offset
(query.order(distanceSorting); query.limit(pageLimit, offset: offset)), which
makes pagination unstable when distances tie; update the ordering to append a
stable secondary sort key (the table primary key such as Merchant.id or
merchantId) before applying limit/offset so ties are deterministic — e.g.,
change the ordering sequence to include distanceSorting then
.order(.ascending("id") or .ascending("merchantId")) so the final query uses
both sorts prior to query.limit(pageLimit, offset: offset).
In `@DashWallet/Sources/UI/Explore` Dash/Merchants &
ATMs/Details/POIDetailsViewController.swift:
- Around line 41-47: The initializer declared as `public
init(pointOfUse:isShowAllHidden:searchRadius:searchCenterCoordinate:currentFilters:)`
has broader access than its containing type; remove the `public` modifier (or
change it to `internal`) so the init's access level matches the parent class
(POIDetailsViewController) and resolves the SwiftLint `lower_acl_than_parent`
warning.
In `@DashWallet/Sources/UI/Explore` Dash/Merchants &
ATMs/List/AllMerchantLocations/AllMerchantLocationsViewController.swift:
- Around line 200-205: In AllMerchantLocationsViewController replace the
hardcoded kDefaultRadius when instantiating POIDetailsViewController with the
view controller’s current searchRadius property so the details view receives the
same search scope; specifically update the POIDetailsViewController initializer
call (where pointOfUse: item, searchRadius: kDefaultRadius,
searchCenterCoordinate: searchCenterCoordinate, currentFilters: currentFilters)
to pass searchRadius instead of kDefaultRadius so POIDetailsViewController uses
the caller’s searchRadius.
In `@DashWallet/Sources/UI/Explore` Dash/Merchants &
ATMs/List/AllMerchantLocations/Components/MerchantCellRow.swift:
- Around line 60-62: The attribute placement for the distanceLabel view property
is breaking SwiftLint rules; move the `@ViewBuilder` attribute onto the same
declaration line for the MerchantCellRow.distanceLabel property (i.e., change
the separate `@ViewBuilder` line so it precedes `private var distanceLabel: some
View` on the same line) and ensure the brace and body remain unchanged so the
file passes the configured SwiftLint `attributes` rule.
In `@DashWallet/Sources/UI/SwiftUI` Components/BottomSheet.swift:
- Around line 88-90: The contentSection property has SwiftLint style violations:
fix the attribute placement for `@ViewBuilder` so it directly precedes the
declaration with no extra blank lines and correct spacing, and normalize spacing
in the preference key declarations (ensure single spaces around let/var and
around colons, and follow the project’s attribute/declaration formatting) —
update the contentSection (uses fillsHeight) and the preference key types (the
preference key declarations on lines ~110-112) to match the lint rules so
attributes are immediately above their declarations and let/var whitespace
follows the configured style.
---
Outside diff comments:
In `@DashWallet/Sources/UI/Explore` Dash/Merchants &
ATMs/Details/POIDetailsViewController.swift:
- Around line 218-234: The UIHostingController's view is being added directly
without attaching the hostingController as a child; update showDetailsView to
call addChild(hostingController) before adding hostingController.view to
contentView and call hostingController.didMove(toParent: self) after activating
constraints so the child lifecycle is propagated correctly (keep
makeDetailsView(), hostingController.view background/translates setup, adding to
contentView, and NSLayoutConstraint.activate as-is, just wrap with
addChild(hostingController) and didMove(toParent: self)).
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 15226cea-c499-41a2-9059-cdd2f7f61f44
⛔ Files ignored due to path filters (21)
DashWallet/Resources/AppAssets.xcassets/Navigation bar/navigationbar-back.imageset/navigationbar-back@1x.pngis excluded by!**/*.pngDashWallet/Resources/AppAssets.xcassets/Navigation bar/navigationbar-back.imageset/navigationbar-back@2x.pngis excluded by!**/*.pngDashWallet/Resources/AppAssets.xcassets/Navigation bar/navigationbar-back.imageset/navigationbar-back@3x.pngis excluded by!**/*.pngDashWallet/Resources/AppAssets.xcassets/Navigation bar/navigationbar-close.imageset/navigationbar-close@1x.pngis excluded by!**/*.pngDashWallet/Resources/AppAssets.xcassets/Navigation bar/navigationbar-close.imageset/navigationbar-close@2x.pngis excluded by!**/*.pngDashWallet/Resources/AppAssets.xcassets/Navigation bar/navigationbar-close.imageset/navigationbar-close@3x.pngis excluded by!**/*.pngDashWallet/Resources/AppAssets.xcassets/Navigation bar/navigationbar-info.imageset/control-info.pngis excluded by!**/*.pngDashWallet/Resources/AppAssets.xcassets/Navigation bar/navigationbar-info.imageset/control-info@2x.pngis excluded by!**/*.pngDashWallet/Resources/AppAssets.xcassets/Navigation bar/navigationbar-info.imageset/control-info@3x.pngis excluded by!**/*.pngDashWallet/Resources/AppAssets.xcassets/Navigation bar/navigationbar-plus.imageset/navigationbar-plus@1x.pngis excluded by!**/*.pngDashWallet/Resources/AppAssets.xcassets/Navigation bar/navigationbar-plus.imageset/navigationbar-plus@2x.pngis excluded by!**/*.pngDashWallet/Resources/AppAssets.xcassets/Navigation bar/navigationbar-plus.imageset/navigationbar-plus@3x.pngis excluded by!**/*.pngDashWallet/Resources/AppAssets.xcassets/Navigation bar/toolbar-back.imageset/control-back.pngis excluded by!**/*.pngDashWallet/Resources/AppAssets.xcassets/Navigation bar/toolbar-back.imageset/control-back@2x.pngis excluded by!**/*.pngDashWallet/Resources/AppAssets.xcassets/Navigation bar/toolbar-back.imageset/control-back@3x.pngis excluded by!**/*.pngDashWallet/Resources/AppAssets.xcassets/Navigation bar/toolbar-close.imageset/control-close.pngis excluded by!**/*.pngDashWallet/Resources/AppAssets.xcassets/Navigation bar/toolbar-close.imageset/control-close@2x.pngis excluded by!**/*.pngDashWallet/Resources/AppAssets.xcassets/Navigation bar/toolbar-close.imageset/control-close@3x.pngis excluded by!**/*.pngDashWallet/Resources/AppAssets.xcassets/Navigation bar/toolbar-plus.imageset/control-plus.pngis excluded by!**/*.pngDashWallet/Resources/AppAssets.xcassets/Navigation bar/toolbar-plus.imageset/control-plus@2x.pngis excluded by!**/*.pngDashWallet/Resources/AppAssets.xcassets/Navigation bar/toolbar-plus.imageset/control-plus@3x.pngis excluded by!**/*.png
📒 Files selected for processing (25)
DashWallet.xcodeproj/project.pbxprojDashWallet/Resources/AppAssets.xcassets/Navigation bar/navigationbar-back.imageset/Contents.jsonDashWallet/Resources/AppAssets.xcassets/Navigation bar/navigationbar-close.imageset/Contents.jsonDashWallet/Resources/AppAssets.xcassets/Navigation bar/navigationbar-info.imageset/Contents.jsonDashWallet/Resources/AppAssets.xcassets/Navigation bar/navigationbar-plus.imageset/Contents.jsonDashWallet/Sources/Models/Explore Dash/ExploreDash.swiftDashWallet/Sources/Models/Explore Dash/Infrastructure/DAO Impl/MerchantDAO.swiftDashWallet/Sources/UI/Explore Dash/Merchants & ATMs/Details/POIDetailsViewController.swiftDashWallet/Sources/UI/Explore Dash/Merchants & ATMs/Details/Views/POIDetailsView.swiftDashWallet/Sources/UI/Explore Dash/Merchants & ATMs/Details/Views/POIDetailsViewModel.swiftDashWallet/Sources/UI/Explore Dash/Merchants & ATMs/List/AllMerchantLocations/AllMerchantLocationsView.swiftDashWallet/Sources/UI/Explore Dash/Merchants & ATMs/List/AllMerchantLocations/AllMerchantLocationsViewController.swiftDashWallet/Sources/UI/Explore Dash/Merchants & ATMs/List/AllMerchantLocations/AllMerchantLocationsViewModel.swiftDashWallet/Sources/UI/Explore Dash/Merchants & ATMs/List/AllMerchantLocations/Components/MerchantCellRow.swiftDashWallet/Sources/UI/Explore Dash/Merchants & ATMs/List/ExplorePointOfUseListViewController.swiftDashWallet/Sources/UI/Explore Dash/Merchants & ATMs/List/MerchantListViewController.swiftDashWallet/Sources/UI/Explore Dash/Merchants & ATMs/List/Model/AllMerchantLocationsDataProvider.swiftDashWallet/Sources/UI/Explore Dash/Merchants & ATMs/List/Model/PointOfUseListModel.swiftDashWallet/Sources/UI/Explore Dash/Merchants & ATMs/List/Views/ExploreMapView.swiftDashWallet/Sources/UI/Explore Dash/Merchants & ATMs/List/Views/ListHandlerView.swiftDashWallet/Sources/UI/Explore Dash/Merchants & ATMs/List/Views/PointOfUseListEmptyResultsView.swiftDashWallet/Sources/UI/Explore Dash/Merchants & ATMs/List/Views/PointOfUseLocationServicePopup.swiftDashWallet/Sources/UI/SwiftUI Components/BottomSheet.swiftDashWallet/Sources/UI/SwiftUI Components/NavigationBar.swiftDashWallet/Sources/UI/SwiftUI Components/SelfSizingSheetModifier.swift
✅ Files skipped from review due to trivial changes (6)
- DashWallet/Resources/AppAssets.xcassets/Navigation bar/navigationbar-close.imageset/Contents.json
- DashWallet/Sources/UI/Explore Dash/Merchants & ATMs/List/Views/ListHandlerView.swift
- DashWallet/Resources/AppAssets.xcassets/Navigation bar/navigationbar-back.imageset/Contents.json
- DashWallet/Sources/UI/Explore Dash/Merchants & ATMs/List/Views/PointOfUseListEmptyResultsView.swift
- DashWallet/Sources/UI/Explore Dash/Merchants & ATMs/List/Views/PointOfUseLocationServicePopup.swift
- DashWallet/Sources/UI/Explore Dash/Merchants & ATMs/List/Views/ExploreMapView.swift
🚧 Files skipped from review as they are similar to previous changes (2)
- DashWallet/Sources/UI/SwiftUI Components/NavigationBar.swift
- DashWallet/Sources/UI/Explore Dash/Merchants & ATMs/List/MerchantListViewController.swift
Comment on lines
+60
to
+62
| @ViewBuilder | ||
| private var distanceLabel: some View { | ||
| if let distanceText { |
There was a problem hiding this comment.
Move @ViewBuilder onto the property declaration line.
Line 61 violates the configured SwiftLint attributes style rule.
Suggested fix
- `@ViewBuilder`
- private var distanceLabel: some View {
+ `@ViewBuilder` private var distanceLabel: some View {As per coding guidelines, Swift files should pass the repo SwiftLint configuration.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| @ViewBuilder | |
| private var distanceLabel: some View { | |
| if let distanceText { | |
| `@ViewBuilder` private var distanceLabel: some View { | |
| if let distanceText { |
🧰 Tools
🪛 SwiftLint (0.63.3)
[Warning] 61-61: Attributes should be on their own lines in functions and types, but on the same line as variables and imports
(attributes)
🤖 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 `@DashWallet/Sources/UI/Explore` Dash/Merchants &
ATMs/List/AllMerchantLocations/Components/MerchantCellRow.swift around lines 60
- 62, The attribute placement for the distanceLabel view property is breaking
SwiftLint rules; move the `@ViewBuilder` attribute onto the same declaration line
for the MerchantCellRow.distanceLabel property (i.e., change the separate
`@ViewBuilder` line so it precedes `private var distanceLabel: some View` on the
same line) and ensure the brace and body remain unchanged so the file passes the
configured SwiftLint `attributes` rule.
Sources: Coding guidelines, Linters/SAST tools
Comment on lines
88
to
+90
| @ViewBuilder | ||
| private var backButtonSection: some View { | ||
| if showBackButton { | ||
| Button { | ||
| onBackButtonPressed?() | ||
| } label: { | ||
| Image(colorScheme == .dark ? "controls-back-dark-mode" : "controls-back") | ||
| .offset(x: -5, y: 5) | ||
| private var contentSection: some View { | ||
| if fillsHeight { |
There was a problem hiding this comment.
Fix SwiftLint style violations in contentSection and preference key declarations.
Line 89 (attributes) and Line 111 (let_var_whitespace) currently violate configured lint rules.
Suggested fix
- `@ViewBuilder`
- private var contentSection: some View {
+ `@ViewBuilder` private var contentSection: some View {
@@
struct BottomSheetHeightPreferenceKey: PreferenceKey {
static var defaultValue: CGFloat = 0
+
static func reduce(value: inout CGFloat, nextValue: () -> CGFloat) {
value = max(value, nextValue())
}
}As per coding guidelines, Swift files must conform to the project’s SwiftLint rules.
Also applies to: 110-112
🧰 Tools
🪛 SwiftLint (0.63.3)
[Warning] 89-89: Attributes should be on their own lines in functions and types, but on the same line as variables and imports
(attributes)
🤖 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 `@DashWallet/Sources/UI/SwiftUI` Components/BottomSheet.swift around lines 88 -
90, The contentSection property has SwiftLint style violations: fix the
attribute placement for `@ViewBuilder` so it directly precedes the declaration
with no extra blank lines and correct spacing, and normalize spacing in the
preference key declarations (ensure single spaces around let/var and around
colons, and follow the project’s attribute/declaration formatting) — update the
contentSection (uses fillsHeight) and the preference key types (the preference
key declarations on lines ~110-112) to match the lint rules so attributes are
immediately above their declarations and let/var whitespace follows the
configured style.
Sources: Coding guidelines, Linters/SAST tools
Why: - keep paginated all-locations results deterministic across pages - preserve the current radius when navigating between all-locations and details What: - add a stable secondary sort for all-locations pagination - validate coordinates in the all-locations count path - pass the current search radius back into merchant details - narrow POIDetailsViewController initializer visibility to internal Validation: - no reliable xcodebuild validation was available in this environment because CoreSimulatorService remained unavailable
There was a problem hiding this comment.
♻️ Duplicate comments (1)
DashWallet/Sources/UI/SwiftUI Components/NavigationBar.swift (1)
146-146:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winMove the
@Environmentattributes onto their own lines.SwiftLint requires attributes with arguments to be on separate lines. Both
@Environment(\.colorScheme)declarations are still on the same line as their property declarations.Suggested fix
At line 146:
- `@Environment`(\.colorScheme) private var colorScheme + `@Environment`(\.colorScheme) + private var colorSchemeAt line 203:
- `@Environment`(\.colorScheme) private var colorScheme + `@Environment`(\.colorScheme) + private var colorSchemeAs per coding guidelines,
**/*.swift: Use SwiftFormat and SwiftLint for Swift code formatting and linting as configured in .swiftformat and .swiftlint.yml files.Also applies to: 203-203
🤖 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 `@DashWallet/Sources/UI/SwiftUI` Components/NavigationBar.swift at line 146, The `@Environment`(\.colorScheme) attribute must be placed on its own line above the property declaration; update both occurrences where you have "`@Environment`(\\.colorScheme) private var colorScheme" so the attribute is on a separate line and the property is declared on the following line (i.e., split the attribute and "private var colorScheme" into two lines) to satisfy SwiftLint/SwiftFormat rules.Source: Coding guidelines
🤖 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.
Duplicate comments:
In `@DashWallet/Sources/UI/SwiftUI` Components/NavigationBar.swift:
- Line 146: The `@Environment`(\.colorScheme) attribute must be placed on its own
line above the property declaration; update both occurrences where you have
"`@Environment`(\\.colorScheme) private var colorScheme" so the attribute is on a
separate line and the property is declared on the following line (i.e., split
the attribute and "private var colorScheme" into two lines) to satisfy
SwiftLint/SwiftFormat rules.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 423d6063-d991-4a77-a8e4-93d4e28d3692
📒 Files selected for processing (6)
DashWallet/Resources/AppAssets.xcassets/Navigation bar/navigationbar-close.imageset/Contents.jsonDashWallet/Resources/AppAssets.xcassets/Navigation bar/navigationbar-plus.imageset/Contents.jsonDashWallet/Sources/Models/Explore Dash/Infrastructure/DAO Impl/MerchantDAO.swiftDashWallet/Sources/UI/Explore Dash/Merchants & ATMs/Details/POIDetailsViewController.swiftDashWallet/Sources/UI/Explore Dash/Merchants & ATMs/List/AllMerchantLocations/AllMerchantLocationsViewController.swiftDashWallet/Sources/UI/SwiftUI Components/NavigationBar.swift
🚧 Files skipped from review as they are similar to previous changes (3)
- DashWallet/Resources/AppAssets.xcassets/Navigation bar/navigationbar-plus.imageset/Contents.json
- DashWallet/Sources/UI/Explore Dash/Merchants & ATMs/List/AllMerchantLocations/AllMerchantLocationsViewController.swift
- DashWallet/Sources/UI/Explore Dash/Merchants & ATMs/Details/POIDetailsViewController.swift
Contributor
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
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
3 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
The “Where to Spend” all-locations flow needed a UI/UX refresh and better state handling (loading/empty/content), while preserving navigation to POI details and existing filters/radius behavior.
What was done?
How Has This Been Tested?
Breaking Changes
None.
Checklist:
For repository code-owners and collaborators only
Summary by CodeRabbit
New Features
Improvements
UX / Polishing