Open
Conversation
Allow embedding UserProfileView in a parent NavigationStack by passing an optional navigationPath binding. When provided, destinations push onto the parent's path instead of an internal stack. - Add externalPath and externalPathBaseCount to UserProfileNavigation - Add navigate(to:) and popToRoot() methods for unified navigation - Extract profileContent to conditionally wrap in NavigationStack - Track isEmbedded in telemetry payload # Conflicts: # Sources/ClerkKitUI/Components/UserProfile/UserProfileView.swift
Move external navigation path configuration from init to taskOnce to ensure the binding is properly captured when the view appears. This fixes an issue where the external path could be stale when passed through initializer state wrappers.
When deleting the last session in embedded mode, we need to also pop the UserProfileView entry itself from the parent's navigation stack, not just the views pushed by the profile flow.
Replace imperative navigation.navigate() calls with a new UserProfileRouter struct that manages push/pop operations via closures. This provides a cleaner API for embedded and standalone navigation scenarios. - Extract routing logic from UserProfileNavigation into UserProfileRouter - Track embedded push count via @State to properly pop to root - Inject router through environment for consistent access
Move navigation path from UserProfileNavigation to local state within UserProfileView, simplifying the navigation class to only manage presentation state. Replace custom EnvironmentKey with @entry macro for cleaner environment value declaration.
Use initialPathCount captured at init to calculate entries to remove during popToRoot, eliminating the need for onChange tracking and embeddedPushCount state management.
Clarifies the caller's intent when popping to root after account deletion. The decision about whether to include the current view in the pop is now made explicitly at the call site based on whether the user still exists and if the account switcher will be presented.
Allow developers to inject custom rows into the root profile screen with flexible placement options. Custom rows can be positioned at section start/end or before/after specific built-in rows.
Replace AnyHashable-based custom row and destination routing with typed Route generics, add UserProfileNavigator for environment-driven navigation, and keep a built-in router for Clerk-owned child views.
Support custom profile rows and destination routing from `UserButton` into `UserProfileView`. Update profile row icon rendering so asset and system icons share consistent sizing and theming.
The public navigator should only allow pushing custom routes. Built-in row navigation is now handled internally through the private navigate(to:) helper method.
The UserProfileNavigator instance was being created on every body evaluation via the computed property. Inline the calls to navigate() directly and construct the navigator only when needed for child views.
Use view modifier API for custom items and destinations
This ensures the initial path count is captured when the view actually appears rather than during init, providing more reliable navigation state tracking.
WalkthroughUser profile UI refactor: UserButton and UserProfile are now generic (Route/Destination), added custom-row APIs and icon typing, replaced path-based UserProfileNavigation with sheet-focused UserProfileSheetNavigation plus UserProfileNavigator/UserProfileBuiltInRouter, and moved multiple views to the new sheet navigation model. Changes
Sequence DiagramsequenceDiagram
participant User
participant UserButton
participant Sheet as ProfileSheet
participant UserProfileView
participant Navigator as UserProfileNavigator
participant BuiltInRouter as UserProfileBuiltInRouter
participant NavStack as ParentNavigationStack
User->>UserButton: tap open profile
UserButton->>Sheet: present sheet (passes customRows/customDestination)
Sheet->>UserProfileView: show profile (embedded? navigationPath)
User->>UserProfileView: tap custom row
UserProfileView->>Navigator: push(route)
Navigator->>BuiltInRouter: translate route/dismiss intent
BuiltInRouter->>NavStack: append/remove entries (if embedded)
NavStack->>UserProfileView: navigationDestination renders Destination
User->>UserProfileView: trigger sign out / delete
UserProfileView->>BuiltInRouter: dismiss(.popToRoot/.exitUserProfile)
BuiltInRouter->>NavStack: adjust path or signal sheet dismissal
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
Sources/ClerkKitUI/Components/UserProfile/BackupCodesView.swift (1)
130-135:⚠️ Potential issue | 🟡 MinorPreview missing
UserProfileSheetNavigationenvironment.This preview doesn't use
clerkPreview()and doesn't injectUserProfileSheetNavigation, which will cause a runtime crash when the "Done" button is tapped (lines 65-70 accessnavigation.presentedAddMfaType).🔧 Suggested fix
`#Preview` { BackupCodesView( backupCodes: ["abc", "def", "ghi", "jkl", "lmn", "opq", "rst", "uvw", "xyz"], mfaType: .authenticatorApp ) + .environment(UserProfileSheetNavigation()) }Or use
.clerkPreview()if the full preview mock setup is desired.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/ClerkKitUI/Components/UserProfile/BackupCodesView.swift` around lines 130 - 135, The preview for BackupCodesView lacks the UserProfileSheetNavigation environment, causing a crash when the "Done" button (code referencing navigation.presentedAddMfaType in BackupCodesView) is tapped; fix by injecting a UserProfileSheetNavigation instance into the Preview—either wrap the Preview BackupCodesView with .environmentObject(UserProfileSheetNavigation()) or replace the preview call with .clerkPreview() to provide the full mock environment so BackupCodesView has the required navigation object.Sources/ClerkKitUI/Components/UserButton/UserButtonAccountSwitcher.swift (1)
171-175:⚠️ Potential issue | 🟡 MinorPreview missing
UserProfileSheetNavigationenvironment.The view reads
@Environment(UserProfileSheetNavigation.self)but the preview does not inject it. This will cause a runtime crash when running the preview.Proposed fix
`#Preview` { UserButtonAccountSwitcher() .clerkPreview() + .environment(UserProfileSheetNavigation()) .environment(\.clerkTheme, .clerk) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/ClerkKitUI/Components/UserButton/UserButtonAccountSwitcher.swift` around lines 171 - 175, The preview for UserButtonAccountSwitcher is missing the required UserProfileSheetNavigation environment and will crash; update the `#Preview` block that creates UserButtonAccountSwitcher() to inject a test/mocked UserProfileSheetNavigation via .environment(UserProfileSheetNavigation.self, <mockInstance>) so the view's `@Environment`(UserProfileSheetNavigation.self) resolves; if UserProfileSheetNavigation is a protocol or type you don’t have an instance for, add a lightweight test/mock implementation (or a no-op wrapper) and pass that instance into the .environment call.Sources/ClerkKitUI/Components/UserProfile/UserProfileSecurityView.swift (1)
88-94:⚠️ Potential issue | 🟡 MinorPreview missing
UserProfileSheetNavigationenvironment.The view reads
@Environment(UserProfileSheetNavigation.self)but the preview does not inject it, which will crash at runtime.Proposed fix
`#Preview` { NavigationStack { UserProfileSecurityView() } .clerkPreview() + .environment(UserProfileSheetNavigation()) .environment(\.clerkTheme, .clerk) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/ClerkKitUI/Components/UserProfile/UserProfileSecurityView.swift` around lines 88 - 94, The preview for UserProfileSecurityView lacks the UserProfileSheetNavigation environment value that the view reads via `@Environment`(UserProfileSheetNavigation.self), which will crash at runtime; update the `#Preview` block that constructs UserProfileSecurityView to inject a suitable preview/mock UserProfileSheetNavigation instance into the environment (e.g., provide a simple/default or mock initializer for UserProfileSheetNavigation) using .environment(UserProfileSheetNavigation.self, /* mock/default instance */) so the preview supplies the dependency when rendering.Sources/ClerkKitUI/Components/UserProfile/UserProfileDeleteAccountConfirmationView.swift (1)
113-117:⚠️ Potential issue | 🟡 MinorPreview missing required environment objects.
The view reads
@Environment(UserProfileSheetNavigation.self)and@Environment(UserProfileBuiltInRouter.self)but the preview does not inject either, which will crash at runtime.Proposed fix
`#Preview` { UserProfileDeleteAccountConfirmationView() .environment(\.clerkTheme, .clerk) .environment(\.locale, .init(identifier: "es")) + .clerkPreview() + .environment(UserProfileSheetNavigation()) + .environment( + UserProfileBuiltInRouter( + push: { _ in }, + dismissAction: { _ in } + ) + ) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/ClerkKitUI/Components/UserProfile/UserProfileDeleteAccountConfirmationView.swift` around lines 113 - 117, The preview is missing the environment objects required by UserProfileDeleteAccountConfirmationView and will crash; update the `#Preview` block to inject mock or default instances for the two environment values read by the view (UserProfileSheetNavigation and UserProfileBuiltInRouter) by adding environment(...) calls that provide simple test implementations or stubs (e.g. a NoOp or preview-only initializer) for UserProfileSheetNavigation and UserProfileBuiltInRouter so the view can render in Xcode previews.
🧹 Nitpick comments (3)
Sources/ClerkKitUI/Components/UserProfile/UserProfileCustomRow.swift (1)
12-45: Consider addingSendableconformance toUserProfileCustomRow.Since all stored properties except
routeare alreadySendable-compatible, andRouteis constrained toHashable, you could addSendableconformance whenRoute: Sendablefor better thread-safety guarantees in concurrent contexts.💡 Optional enhancement
-public struct UserProfileCustomRow<Route: Hashable> { +public struct UserProfileCustomRow<Route: Hashable>: Sendable where Route: Sendable {Alternatively, use a conditional conformance extension:
extension UserProfileCustomRow: Sendable where Route: Sendable {}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/ClerkKitUI/Components/UserProfile/UserProfileCustomRow.swift` around lines 12 - 45, Add Sendable conformance for UserProfileCustomRow by making it conditionally Sendable when its generic Route is Sendable; update the declaration or add a conditional conformance extension for UserProfileCustomRow (e.g., extension UserProfileCustomRow: Sendable where Route: Sendable) so the struct is Sendable in concurrent contexts while preserving existing behavior and generic constraints.Sources/ClerkKitUI/Components/UserButton/UserProfileRowView.swift (1)
8-8: Remove unusedimport Foundation.The
Foundationimport appears unnecessary as no Foundation-specific types are used in this file. SwiftUI already re-exports the Foundation types commonly needed.🧹 Suggested fix
-import Foundation import SwiftUI🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/ClerkKitUI/Components/UserButton/UserProfileRowView.swift` at line 8, Remove the unused import by deleting the lone "import Foundation" statement in UserProfileRowView.swift (the file containing the UserProfileRowView view); SwiftUI already provides the needed Foundation types so simply remove the import Foundation line to eliminate the unused import warning.Sources/ClerkKitUI/Components/UserProfile/UserProfileView.swift (1)
458-474: Consider logging whencustomDestinationis nil for a custom route.When
view(for:)receives a.custom(route)butcustomDestinationis nil, it silently returnsEmptyView(). This could make debugging difficult if a developer forgets to call.userProfileDestination(...).Proposed diagnostic
case .custom(let route): if let customDestination { customDestination(route) } else { + `#if` DEBUG + let _ = print("[UserProfileView] No customDestination provided for route: \(route)") + `#endif` EmptyView() }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/ClerkKitUI/Components/UserProfile/UserProfileView.swift` around lines 458 - 474, The view(for:) switch silently returns EmptyView when handling a .custom(route) and customDestination is nil; update view(for:) in UserProfileView to log or surface a diagnostic when customDestination is nil for a .custom route (e.g., use Logger/print/assertionFailure) so developers see that UserProfileNavigationDestination<Route>.custom had no handler; ensure the message includes the route value and a clear hint to call .userProfileDestination(...), but keep the existing EmptyView fallback for runtime safety.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@Sources/ClerkKitUI/Components/UserButton/UserButtonAccountSwitcher.swift`:
- Around line 171-175: The preview for UserButtonAccountSwitcher is missing the
required UserProfileSheetNavigation environment and will crash; update the
`#Preview` block that creates UserButtonAccountSwitcher() to inject a test/mocked
UserProfileSheetNavigation via .environment(UserProfileSheetNavigation.self,
<mockInstance>) so the view's `@Environment`(UserProfileSheetNavigation.self)
resolves; if UserProfileSheetNavigation is a protocol or type you don’t have an
instance for, add a lightweight test/mock implementation (or a no-op wrapper)
and pass that instance into the .environment call.
In `@Sources/ClerkKitUI/Components/UserProfile/BackupCodesView.swift`:
- Around line 130-135: The preview for BackupCodesView lacks the
UserProfileSheetNavigation environment, causing a crash when the "Done" button
(code referencing navigation.presentedAddMfaType in BackupCodesView) is tapped;
fix by injecting a UserProfileSheetNavigation instance into the Preview—either
wrap the Preview BackupCodesView with
.environmentObject(UserProfileSheetNavigation()) or replace the preview call
with .clerkPreview() to provide the full mock environment so BackupCodesView has
the required navigation object.
In
`@Sources/ClerkKitUI/Components/UserProfile/UserProfileDeleteAccountConfirmationView.swift`:
- Around line 113-117: The preview is missing the environment objects required
by UserProfileDeleteAccountConfirmationView and will crash; update the `#Preview`
block to inject mock or default instances for the two environment values read by
the view (UserProfileSheetNavigation and UserProfileBuiltInRouter) by adding
environment(...) calls that provide simple test implementations or stubs (e.g. a
NoOp or preview-only initializer) for UserProfileSheetNavigation and
UserProfileBuiltInRouter so the view can render in Xcode previews.
In `@Sources/ClerkKitUI/Components/UserProfile/UserProfileSecurityView.swift`:
- Around line 88-94: The preview for UserProfileSecurityView lacks the
UserProfileSheetNavigation environment value that the view reads via
`@Environment`(UserProfileSheetNavigation.self), which will crash at runtime;
update the `#Preview` block that constructs UserProfileSecurityView to inject a
suitable preview/mock UserProfileSheetNavigation instance into the environment
(e.g., provide a simple/default or mock initializer for
UserProfileSheetNavigation) using .environment(UserProfileSheetNavigation.self,
/* mock/default instance */) so the preview supplies the dependency when
rendering.
---
Nitpick comments:
In `@Sources/ClerkKitUI/Components/UserButton/UserProfileRowView.swift`:
- Line 8: Remove the unused import by deleting the lone "import Foundation"
statement in UserProfileRowView.swift (the file containing the
UserProfileRowView view); SwiftUI already provides the needed Foundation types
so simply remove the import Foundation line to eliminate the unused import
warning.
In `@Sources/ClerkKitUI/Components/UserProfile/UserProfileCustomRow.swift`:
- Around line 12-45: Add Sendable conformance for UserProfileCustomRow by making
it conditionally Sendable when its generic Route is Sendable; update the
declaration or add a conditional conformance extension for UserProfileCustomRow
(e.g., extension UserProfileCustomRow: Sendable where Route: Sendable) so the
struct is Sendable in concurrent contexts while preserving existing behavior and
generic constraints.
In `@Sources/ClerkKitUI/Components/UserProfile/UserProfileView.swift`:
- Around line 458-474: The view(for:) switch silently returns EmptyView when
handling a .custom(route) and customDestination is nil; update view(for:) in
UserProfileView to log or surface a diagnostic when customDestination is nil for
a .custom route (e.g., use Logger/print/assertionFailure) so developers see that
UserProfileNavigationDestination<Route>.custom had no handler; ensure the
message includes the route value and a clear hint to call
.userProfileDestination(...), but keep the existing EmptyView fallback for
runtime safety.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: ab1d7e74-bca2-459b-8fc7-c65a3378a3aa
📒 Files selected for processing (14)
Sources/ClerkKitUI/Components/UserButton/UserButton.swiftSources/ClerkKitUI/Components/UserButton/UserButtonAccountSwitcher.swiftSources/ClerkKitUI/Components/UserButton/UserProfileRowView.swiftSources/ClerkKitUI/Components/UserProfile/BackupCodesView.swiftSources/ClerkKitUI/Components/UserProfile/UserProfileAddMfaView.swiftSources/ClerkKitUI/Components/UserProfile/UserProfileCustomRow.swiftSources/ClerkKitUI/Components/UserProfile/UserProfileDeleteAccountConfirmationView.swiftSources/ClerkKitUI/Components/UserProfile/UserProfileMfaAddSmsView.swiftSources/ClerkKitUI/Components/UserProfile/UserProfileMfaAddTotpView.swiftSources/ClerkKitUI/Components/UserProfile/UserProfileMfaSection.swiftSources/ClerkKitUI/Components/UserProfile/UserProfileNavigation.swiftSources/ClerkKitUI/Components/UserProfile/UserProfileSecurityView.swiftSources/ClerkKitUI/Components/UserProfile/UserProfileView.swiftSources/ClerkKitUI/Extensions/View+PreviewMocks.swift
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Sources/ClerkKitUI/Components/UserProfile/UserProfileDeleteAccountConfirmationView.swift (1)
94-107:⚠️ Potential issue | 🟠 MajorAdd
@MainActorannotation todeleteAccount()method.After the suspension point at line 98, lines 101–107 update SwiftUI state and environment properties (
dismiss(),builtInRouter.dismiss(),navigation.accountSwitcherIsPresented,self.error). These mutations must execute on the main thread, but async methods do not automatically inherit the main actor context after suspension. Explicit@MainActorisolation is required here.Suggested fix
extension UserProfileDeleteAccountConfirmationView { + `@MainActor` func deleteAccount() async { guard let user else { return }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/ClerkKitUI/Components/UserProfile/UserProfileDeleteAccountConfirmationView.swift` around lines 94 - 107, The async method deleteAccount() performs UI/state mutations after an await (user.delete()) — calls like dismiss(), builtInRouter.dismiss(...), setting navigation.accountSwitcherIsPresented, and assigning self.error must run on the main actor; annotate the function with `@MainActor` to ensure main-thread isolation so those updates execute safely after suspension.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In
`@Sources/ClerkKitUI/Components/UserProfile/UserProfileDeleteAccountConfirmationView.swift`:
- Around line 94-107: The async method deleteAccount() performs UI/state
mutations after an await (user.delete()) — calls like dismiss(),
builtInRouter.dismiss(...), setting navigation.accountSwitcherIsPresented, and
assigning self.error must run on the main actor; annotate the function with
`@MainActor` to ensure main-thread isolation so those updates execute safely after
suspension.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: f010a528-d69e-4296-a7ae-36a08743c42a
📒 Files selected for processing (6)
Sources/ClerkKitUI/Components/UserButton/UserButtonAccountSwitcher.swiftSources/ClerkKitUI/Components/UserButton/UserProfileRowView.swiftSources/ClerkKitUI/Components/UserProfile/BackupCodesView.swiftSources/ClerkKitUI/Components/UserProfile/UserProfileDeleteAccountConfirmationView.swiftSources/ClerkKitUI/Components/UserProfile/UserProfileSecurityView.swiftSources/ClerkKitUI/Components/UserProfile/UserProfileView.swift
✅ Files skipped from review due to trivial changes (1)
- Sources/ClerkKitUI/Components/UserProfile/UserProfileSecurityView.swift
🚧 Files skipped from review as they are similar to previous changes (4)
- Sources/ClerkKitUI/Components/UserProfile/BackupCodesView.swift
- Sources/ClerkKitUI/Components/UserButton/UserButtonAccountSwitcher.swift
- Sources/ClerkKitUI/Components/UserButton/UserProfileRowView.swift
- Sources/ClerkKitUI/Components/UserProfile/UserProfileView.swift
Separate built-in and custom route navigation paths to allow custom routes to use a distinct navigation destination. Extract header view and custom row helpers into separate files for better organization.
There was a problem hiding this comment.
🧹 Nitpick comments (4)
Sources/ClerkKitUI/Components/UserProfile/UserProfileHeaderView.swift (2)
19-20: Consider checking for emptyfullNameconsistently with username handling.Line 49 checks
!username.isEmptyTrimmedbefore displaying the username, butfullNameis only checked fornil. IffullNamecan be an empty or whitespace-only string, the header may render an empty text view.Proposed fix
- let fullName = user.fullName - let hasFullName = fullName != nil + let fullName = user.fullName?.isEmptyTrimmed == false ? user.fullName : nil + let hasFullName = fullName != nil🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/ClerkKitUI/Components/UserProfile/UserProfileHeaderView.swift` around lines 19 - 20, The code only checks fullName for nil via fullName and hasFullName; change the logic to treat empty or whitespace-only names the same as username’s check by using the same trimmed-empty check (e.g., replace hasFullName calculation to use fullName?.isEmptyTrimmed == false) and ensure the UI branch that displays the full name in UserProfileHeaderView uses that updated hasFullName (or directly checks fullName?.isEmptyTrimmed) so an empty/whitespace fullName does not render an empty text view.
62-68: The.simultaneousGesture(TapGesture())appears unnecessary.This pattern is typically used to allow gesture passthrough to parent views, but for a standalone button calling a callback, it doesn't seem to serve a purpose. Consider removing it unless there's a specific gesture conflict being addressed.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/ClerkKitUI/Components/UserProfile/UserProfileHeaderView.swift` around lines 62 - 68, Remove the unnecessary simultaneous gesture attached to the Edit profile Button in UserProfileHeaderView: delete the .simultaneousGesture(TapGesture()) modifier so the Button remains as Button { onUpdateProfile() } label: { Text("Edit profile", bundle: .module) } with the existing .buttonStyle(.secondary(config: .init(size: .small))). Ensure the onUpdateProfile callback remains wired and run the UI/interaction check to confirm no gesture conflicts exist elsewhere.Sources/ClerkKitUI/Components/UserProfile/UserProfileCustomRow.swift (1)
12-45: Consider addingSendableconformance for concurrency safety.
UserProfileCustomRowcontainsSendable-conforming types (LocalizedStringKey,UserProfileRowIcon,Bundle?,UserProfileCustomRowPlacement) except forRoute. Adding a conditionalSendableconformance would enable safe usage across concurrency boundaries.Proposed addition
public struct UserProfileCustomRow<Route: Hashable> { // ... } + +extension UserProfileCustomRow: Sendable where Route: Sendable {}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/ClerkKitUI/Components/UserProfile/UserProfileCustomRow.swift` around lines 12 - 45, UserProfileCustomRow should declare a conditional Sendable conformance so it can be used safely across concurrency boundaries; add a public extension like "extension UserProfileCustomRow: Sendable where Route: Sendable" (or equivalent declaration) to the type so the compiler treats the struct as Sendable only when the generic Route is Sendable, referencing the struct name UserProfileCustomRow and the generic parameter Route and leaving existing properties (title, icon, bundle, placement) unchanged.Sources/ClerkKitUI/Components/UserProfile/UserProfileView.swift (1)
323-339: Consider extracting duplicate environment injection into a helper.The same environment values (
sheetNavigation,codeLimiter,UserProfileNavigator,UserProfileBuiltInRouter) are injected in two places: lines 163-176 and lines 325-338. This duplication could lead to inconsistencies if one is updated without the other.Proposed helper
private func withNavigationEnvironment<Content: View>( `@ViewBuilder` _ content: () -> Content ) -> some View { content() .environment(sheetNavigation) .environment(codeLimiter) .environment( UserProfileNavigator( push: navigateToCustom, popToRoot: { dismissAction(.popToRoot) } ) ) .environment( UserProfileBuiltInRouter( push: navigateToBuiltIn, dismissAction: dismissAction ) ) }Then use it in both
.navigationDestinationclosures.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/ClerkKitUI/Components/UserProfile/UserProfileView.swift` around lines 323 - 339, Extract the repeated environment injection into a helper like withNavigationEnvironment that takes a `@ViewBuilder` content closure and applies .environment(sheetNavigation), .environment(codeLimiter), .environment(UserProfileNavigator(push: navigateToCustom, popToRoot: { dismissAction(.popToRoot) })), and .environment(UserProfileBuiltInRouter(push: navigateToBuiltIn, dismissAction: dismissAction)); then replace the duplicated environment chains around view(for:) in both places with a call to withNavigationEnvironment { view(for: destination) } (and the corresponding call where the other duplicate exists) so all four environments are applied from one centralized helper.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@Sources/ClerkKitUI/Components/UserProfile/UserProfileCustomRow.swift`:
- Around line 12-45: UserProfileCustomRow should declare a conditional Sendable
conformance so it can be used safely across concurrency boundaries; add a public
extension like "extension UserProfileCustomRow: Sendable where Route: Sendable"
(or equivalent declaration) to the type so the compiler treats the struct as
Sendable only when the generic Route is Sendable, referencing the struct name
UserProfileCustomRow and the generic parameter Route and leaving existing
properties (title, icon, bundle, placement) unchanged.
In `@Sources/ClerkKitUI/Components/UserProfile/UserProfileHeaderView.swift`:
- Around line 19-20: The code only checks fullName for nil via fullName and
hasFullName; change the logic to treat empty or whitespace-only names the same
as username’s check by using the same trimmed-empty check (e.g., replace
hasFullName calculation to use fullName?.isEmptyTrimmed == false) and ensure the
UI branch that displays the full name in UserProfileHeaderView uses that updated
hasFullName (or directly checks fullName?.isEmptyTrimmed) so an empty/whitespace
fullName does not render an empty text view.
- Around line 62-68: Remove the unnecessary simultaneous gesture attached to the
Edit profile Button in UserProfileHeaderView: delete the
.simultaneousGesture(TapGesture()) modifier so the Button remains as Button {
onUpdateProfile() } label: { Text("Edit profile", bundle: .module) } with the
existing .buttonStyle(.secondary(config: .init(size: .small))). Ensure the
onUpdateProfile callback remains wired and run the UI/interaction check to
confirm no gesture conflicts exist elsewhere.
In `@Sources/ClerkKitUI/Components/UserProfile/UserProfileView.swift`:
- Around line 323-339: Extract the repeated environment injection into a helper
like withNavigationEnvironment that takes a `@ViewBuilder` content closure and
applies .environment(sheetNavigation), .environment(codeLimiter),
.environment(UserProfileNavigator(push: navigateToCustom, popToRoot: {
dismissAction(.popToRoot) })), and .environment(UserProfileBuiltInRouter(push:
navigateToBuiltIn, dismissAction: dismissAction)); then replace the duplicated
environment chains around view(for:) in both places with a call to
withNavigationEnvironment { view(for: destination) } (and the corresponding call
where the other duplicate exists) so all four environments are applied from one
centralized helper.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 8a2c99ac-44a1-4a6f-987e-86766aa2e4b9
📒 Files selected for processing (4)
Sources/ClerkKitUI/Components/UserProfile/UserProfileCustomRow.swiftSources/ClerkKitUI/Components/UserProfile/UserProfileHeaderView.swiftSources/ClerkKitUI/Components/UserProfile/UserProfileNavigation.swiftSources/ClerkKitUI/Components/UserProfile/UserProfileView.swift
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
1 participant
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.
Summary by CodeRabbit
New Features
Refactor