Replace Lottie connect button with toggle switch#140
Conversation
- Add VPNToggleView: custom pill toggle with spring animation and pulse during connecting/disconnecting states - Optimistic UI: thumb moves instantly on tap, pulse starts before OS confirms - Simplify iOSConnectionView: monotone BgMenu background, centered layout, SECURED/NOT SECURED status label, hostname/IP grouped under toggle
|
Note Currently processing new changes in this PR. This may take a few minutes, please wait... ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughA new ChangesConnection UI Modernization with IPv6 Support
Sequence Diagram(s)sequenceDiagram
participant User as User tap
participant Toggle as VPNToggleView
participant ViewModel as MainViewModel
participant Cache as ProfileConnectionCache
participant Poll as Polling details
User->>Toggle: tap toggle
Toggle->>Toggle: set optimisticIsOn immediately
Toggle->>ViewModel: call onConnect() / onDisconnect()
ViewModel->>Poll: await details.vpnState change
Poll-->>ViewModel: receive updated vpnState + ipv6
ViewModel->>Cache: persist ipv6 to cache
ViewModel->>Toggle: vpnState onChange clears optimisticIsOn
Toggle-->>User: toggle settled to final state
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~28 minutes Possibly Related PRs
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 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@NetBird/Source/App/Views/iOS/iOSConnectionView.swift`:
- Around line 104-106: The `.smooth` animation curve is not compatible with the
iOS 15 deployment target, as it was only introduced in iOS 17. In
NetBird/Source/App/Views/iOS/iOSConnectionView.swift at lines 104-106, replace
the `.smooth` animation in the withAnimation call with a compatible animation
curve such as `.easeInOut` or `.default`. Apply the same change to all other
instances of `.smooth` animation in the file, including the ones at lines
121-123, to ensure compatibility across all iOS 15+ devices.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 264cf8ae-3b6b-4fd5-9420-fc8d1568567f
📒 Files selected for processing (3)
NetBird.xcodeproj/project.pbxprojNetBird/Source/App/Views/Components/VPNToggleView.swiftNetBird/Source/App/Views/iOS/iOSConnectionView.swift
|
/testflight |
|
TestFlight builds uploaded |
|
/testflight |
|
TestFlight builds uploaded |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
NetBird/Source/App/Views/iOS/iOSConnectionView.swift (1)
82-86: 💤 Low valueChevron rotation appears inverted from standard disclosure pattern.
When
showAddressDetailsisfalse, the chevron points up (180° rotation); whentrue, it points down (0°). Standard UX typically shows a down-pointing chevron for "tap to expand" and up-pointing for "tap to collapse."Suggested fix to follow standard disclosure pattern
Image(systemName: "chevron.down") .font(.system(size: 12, weight: .semibold)) .foregroundColor(Color("TextSecondary")) - .rotationEffect(.degrees(showAddressDetails ? 0 : 180)) + .rotationEffect(.degrees(showAddressDetails ? 180 : 0))🤖 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 `@NetBird/Source/App/Views/iOS/iOSConnectionView.swift` around lines 82 - 86, The chevron rotation logic in the rotationEffect modifier is inverted from the standard disclosure pattern. In the iOSConnectionView file, the Image with systemName "chevron.down" currently rotates 0 degrees when showAddressDetails is true and 180 degrees when false, which is opposite to standard UX expectations. Swap the rotation values in the rotationEffect call so that when showAddressDetails is true (expanded state), the chevron rotates 180 degrees, and when showAddressDetails is false (collapsed state), it rotates 0 degrees.
🤖 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 `@NetBird/Source/App/Views/iOS/iOSConnectionView.swift`:
- Line 53: The font family names in the .font(.custom()) calls do not exactly
match the registered variable font names, causing silent fallback to the system
font. Update the font family parameter in the .custom() method at line 53 from
"InterVariable" to "Inter Variable" (adding a space), and update the font family
parameter in the second .custom() call at line 59 from "JetBrainsMono-Regular"
to "JetBrains Mono". SwiftUI requires exact matches for font family names,
including spaces and case sensitivity.
---
Nitpick comments:
In `@NetBird/Source/App/Views/iOS/iOSConnectionView.swift`:
- Around line 82-86: The chevron rotation logic in the rotationEffect modifier
is inverted from the standard disclosure pattern. In the iOSConnectionView file,
the Image with systemName "chevron.down" currently rotates 0 degrees when
showAddressDetails is true and 180 degrees when false, which is opposite to
standard UX expectations. Swap the rotation values in the rotationEffect call so
that when showAddressDetails is true (expanded state), the chevron rotates 180
degrees, and when showAddressDetails is false (collapsed state), it rotates 0
degrees.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6c82acf9-a11e-4345-906d-c88262db6a52
⛔ Files ignored due to path filters (2)
NetBird/Fonts/inter-variable.ttfis excluded by!**/*.ttfNetBird/Fonts/jetbrains-mono-variable.ttfis excluded by!**/*.ttf
📒 Files selected for processing (5)
NetBird.xcodeproj/project.pbxprojNetBird/Info.plistNetBird/Source/App/ViewModels/MainViewModel.swiftNetBird/Source/App/Views/iOS/iOSConnectionView.swiftNetbirdKit/ProfileConnectionCache.swift
✅ Files skipped from review due to trivial changes (1)
- NetBird/Info.plist
|
/testflight |
|
TestFlight builds uploaded |
|
/testflight |
|
TestFlight builds uploaded |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
NetBird/Source/App/ViewModels/MainViewModel.swift (1)
498-499: 💤 Low valueConsider including IPv6 in widget state for completeness.
The widget state update includes
ipandfqdnbut notipv6. If the widget has space to display IPv6 information (similar to the main connection view), consider adding:userDefaults?.set(ipv6, forKey: GlobalConstants.keyWidgetIPv6)If the widget intentionally excludes IPv6 due to space constraints, this can be ignored.
🤖 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 `@NetBird/Source/App/ViewModels/MainViewModel.swift` around lines 498 - 499, The widget state update is missing IPv6 information. After the lines where ip and fqdn are stored to userDefaults using userDefaults?.set with GlobalConstants.keyWidgetIP and GlobalConstants.keyWidgetFQDN respectively, add a corresponding line to store ipv6 to userDefaults using GlobalConstants.keyWidgetIPv6 as the key to ensure the widget has access to complete network information (unless IPv6 is intentionally excluded due to space constraints in the widget display).
🤖 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.
Nitpick comments:
In `@NetBird/Source/App/ViewModels/MainViewModel.swift`:
- Around line 498-499: The widget state update is missing IPv6 information.
After the lines where ip and fqdn are stored to userDefaults using
userDefaults?.set with GlobalConstants.keyWidgetIP and
GlobalConstants.keyWidgetFQDN respectively, add a corresponding line to store
ipv6 to userDefaults using GlobalConstants.keyWidgetIPv6 as the key to ensure
the widget has access to complete network information (unless IPv6 is
intentionally excluded due to space constraints in the widget display).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 67a9371f-ab9c-43d3-bc5b-630729d50439
📒 Files selected for processing (4)
NetBird.xcodeproj/project.pbxprojNetBird/Source/App/ViewModels/MainViewModel.swiftNetBird/Source/App/Views/Components/VPNToggleView.swiftNetBird/Source/App/Views/iOS/iOSConnectionView.swift
🚧 Files skipped from review as they are similar to previous changes (3)
- NetBird/Source/App/Views/Components/VPNToggleView.swift
- NetBird.xcodeproj/project.pbxproj
- NetBird/Source/App/Views/iOS/iOSConnectionView.swift
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
|
/testflight |
|
Build failed (iOS, tvOS) |
|
/testflight |
|
TestFlight builds uploaded |
Description
Summary
VPNToggleView), matching the desktop app's visual directionBgMenu), centered layout with SECURED / NOT SECURED status label and hostname/IP grouped under the toggleNetBirdSDK.xcframeworkfrom the currentnetbird-coresubmodule commitDetails
VPNToggleView— orange/gray pill toggle with spring thumb animation and pulsing track during connecting/disconnectingiOSConnectionViewrewritten: removed Lottie dependency and two-tone background image overlay, all connect/disconnect logic (buttonLock,connect(),close()) preserved unchangedTest plan
buttonLockis activeSummary by CodeRabbit