Update UI for Settings and associated views #143
Conversation
WalkthroughThis pull request represents a significant refactoring of the LDKNodeMonday application, focusing on restructuring the project's architecture and views. The changes primarily involve renaming the "Profile" section to "Settings," updating various file references, and introducing new views and view models. Several existing views have been removed, streamlining the user interface for managing Lightning Network node settings, peers, channels, and recovery phrases. Additionally, modifications to error handling and code readability have been implemented in several service files. Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
🧹 Nitpick comments (21)
LDKNodeMonday/Service/Lightning Service/LightningNodeService.swift (1)
Line range hint
1-391: Consider implementing retry mechanisms for network operations.Given that this is a network-dependent service handling Lightning Network operations, it would be beneficial to add retry mechanisms for network-related operations.
Consider:
- Adding retry logic for connection attempts
- Implementing exponential backoff for failed operations
- Adding circuit breaker patterns for external service calls
Would you like me to provide an example implementation of these patterns?
LDKNodeMonday/View/Settings/Peers/DisconnectView.swift (4)
25-25: Fix comment formattingAdd a space after the comment slashes to comply with SwiftLint guidelines.
- //.frame(width: 100) + // .frame(width: 100)🧰 Tools
🪛 SwiftLint (0.57.0)
[Warning] 25-25: Prefer at least one space after slashes for comments
(comment_spacing)
20-34: Enhance node ID display for better UXConsider these improvements for better user experience:
- Add a copy button for the node ID
- Use a monospace font for better readability
- Consider showing a shortened version with expansion capability
List { VStack(alignment: .leading) { Text("Node ID") .font(.subheadline.weight(.medium)) - Text(viewModel.nodeId.description) - .truncationMode(.middle) - .lineLimit(1) - .font(.caption) - .foregroundColor(.secondary) + HStack { + Text(viewModel.nodeId.description) + .truncationMode(.middle) + .lineLimit(1) + .font(.custom("Menlo", size: 12)) + .foregroundColor(.secondary) + Button(action: { + UIPasteboard.general.string = viewModel.nodeId.description + }) { + Image(systemName: "doc.on.doc") + .font(.caption) + } + } }🧰 Tools
🪛 SwiftLint (0.57.0)
[Warning] 25-25: Prefer at least one space after slashes for comments
(comment_spacing)
80-88: Enhance preview with multiple scenariosConsider adding more preview scenarios to test different states:
- Error state
- Different node ID lengths
- Dark/light mode variations
#if DEBUG #Preview { - DisconnectView( - viewModel: .init( - nodeId: "03e39c737a691931dac0f9f9ee803f2ab08f7fd3bbb25ec08d9b8fdb8f51d3a8db" - ) - ) + Group { + DisconnectView( + viewModel: .init( + nodeId: "03e39c737a691931dac0f9f9ee803f2ab08f7fd3bbb25ec08d9b8fdb8f51d3a8db" + ) + ) + DisconnectView( + viewModel: { + let vm = DisconnectViewModel(nodeId: "short_id") + vm.disconnectViewError = .init(title: "Error", detail: "Test error") + return vm + }() + ) + .preferredColorScheme(.dark) + } } #endif
44-56: Add confirmation dialog for disconnect actionSince disconnecting a peer is a critical operation, consider adding a confirmation dialog before proceeding with the disconnect action.
Button { + guard !viewModel.isDisconnecting else { return } + viewModel.showingConfirmationDialog = true +} label: { + Text("Disconnect") + .padding() +} +.confirmationDialog( + "Are you sure you want to disconnect this peer?", + isPresented: $viewModel.showingConfirmationDialog, + titleVisibility: .visible +) { Button { viewModel.disconnect() self.presentationMode.wrappedValue.dismiss() } label: { - Text("Disconnect") - .padding() + Text("Confirm") } -} + Button("Cancel", role: .cancel) { } +}LDKNodeMonday/View/Settings/Danger/SeedView.swift (4)
79-87: Improve error handling specificityThe current error handling uses generic fallbacks for unknown errors. Consider:
- Using more specific error types
- Providing actionable error messages
- Adding retry functionality for recoverable errors
- .alert(isPresented: $showingSeedViewErrorAlert) { - Alert( - title: Text(viewModel.seedViewError?.title ?? "Unknown"), - message: Text(viewModel.seedViewError?.detail ?? ""), - dismissButton: .default(Text("OK")) { - viewModel.seedViewError = nil - } - ) - } + .alert(isPresented: $showingSeedViewErrorAlert) { + Alert( + title: Text(viewModel.seedViewError?.title ?? "Error Accessing Recovery Phrase"), + message: Text(viewModel.seedViewError?.detail ?? "Unable to access your recovery phrase. Please ensure your device is secure and try again."), + primaryButton: .default(Text("Retry")) { + viewModel.getSeed() + }, + secondaryButton: .cancel(Text("Dismiss")) { + viewModel.seedViewError = nil + } + ) + }
72-75: Enhance accessibility supportWhile the current implementation includes dynamic type support, consider adding:
- VoiceOver labels for critical UI elements
- Haptic feedback for important actions
- Accessibility hints for better context
- }.dynamicTypeSize(...DynamicTypeSize.accessibility1) + }.dynamicTypeSize(...DynamicTypeSize.accessibility1) + .accessibilityElement(children: .contain) + .accessibilityLabel("Recovery Phrase View") + .accessibilityHint("Contains sensitive recovery information. Please ensure privacy before proceeding.")
92-96: Enhance preview provider with multiple statesConsider adding preview cases for different view states to aid development and testing:
- Initial warning state
- Shown recovery phrase state
- Error state
- Copying state
#if DEBUG - #Preview { - SeedView(viewModel: .init()) + #Preview("Warning State") { + SeedView(viewModel: .init()) + } + + #Preview("Shown Recovery Phrase") { + let vm = SeedViewModel() + let view = SeedView(viewModel: vm) + view.showRecoveryPhrase = true + return view + } + + #Preview("Error State") { + let vm = SeedViewModel() + vm.seedViewError = SeedViewError(title: "Error", detail: "Test error message") + return SeedView(viewModel: vm) } #endif
67-67: Fix comment spacingAdd a space after the double slash for consistent comment formatting.
- //Spacer() + // Spacer()🧰 Tools
🪛 SwiftLint (0.57.0)
[Warning] 67-67: Prefer at least one space after slashes for comments
(comment_spacing)
LDKNodeMonday/View/Settings/Peers/PeersListView.swift (1)
42-53: Consider using a more accessible color scheme for connection statusThe connection status indicators only use color secondary which might not be sufficient for users with color vision deficiencies. Consider adding distinct icons or patterns to improve accessibility.
- peer.isConnected - ? HStack(alignment: .firstTextBaseline, spacing: 2) { - Text("Connected") - Image(systemName: "checkmark") - }.font(.caption) - .foregroundColor(.secondary) - : HStack { - Text("Not Connected") - Image(systemName: "xmark") - }.font(.caption) - .foregroundColor(.secondary) + peer.isConnected + ? HStack(alignment: .firstTextBaseline, spacing: 2) { + Text("Connected") + Image(systemName: "checkmark.circle.fill") + }.font(.caption) + .foregroundColor(.green) + : HStack { + Text("Not Connected") + Image(systemName: "xmark.circle.fill") + }.font(.caption) + .foregroundColor(.red)🧰 Tools
🪛 SwiftLint (0.57.0)
[Warning] 43-43: Using ternary to call Void functions should be avoided
(void_function_in_ternary)
LDKNodeMonday/View/Settings/Peers/PeerView.swift (1)
50-52: Address SwiftLint warnings about unused closure parametersThe
onChangemodifiers have unusedoldValueparameters.- ).onChange(of: viewModel.nodeId) { oldValue, newValue in + ).onChange(of: viewModel.nodeId) { _, newValue in - ).onChange(of: viewModel.address) { oldValue, newValue in + ).onChange(of: viewModel.address) { _, newValue inAlso applies to: 71-73
🧰 Tools
🪛 SwiftLint (0.57.0)
[Warning] 50-50: Unused parameter in a closure should be replaced with _
(unused_closure_parameter)
LDKNodeMonday/View/Settings/Channel/ChannelsListView.swift (3)
38-79: Consider extracting channel item into a separate view component.The channel item layout is complex and could benefit from being extracted into a separate view component for better maintainability and reusability.
Consider creating a
ChannelListItemView:struct ChannelListItemView: View { let channel: Channel let alias: String? var body: some View { HStack(alignment: .center, spacing: 15) { // Current layout code... } } }Also consider adding formatting for the capacity values to improve readability:
-Text("Send \(channel.outboundCapacityMsat/1000) sats ") +Text("Send \(NumberFormatter.localizedString(from: NSNumber(value: channel.outboundCapacityMsat/1000), number: .decimal)) sats")
96-98: Enhance accessibility with VoiceOver support.While dynamic type support is implemented, consider adding VoiceOver descriptions for better accessibility.
Add accessibility labels to improve VoiceOver support:
Circle() .stroke(lineWidth: 2) .frame(width: 40, height: 40) + .accessibilityLabel("Channel icon") Text("\(channel.channelValueSats) sats") + .accessibilityLabel("Channel value: \(channel.channelValueSats) satoshis")
101-107: Enhance toolbar button clarity.Consider using a more descriptive label for the Add button to improve user understanding.
-Text("Add") +Text("Add Channel")LDKNodeMonday/View/Settings/Channel/ChannelDetailView.swift (2)
26-62: Enhance clipboard operation error handling.The clipboard operation should handle potential failures and provide appropriate feedback.
Button { - UIPasteboard.general.string = property.value + do { + try UIPasteboard.general.setString(property.value) + lastCopiedItemId = property.name + } catch { + // Handle clipboard error + print("Failed to copy to clipboard: \(error)") + } - lastCopiedItemId = property.name DispatchQueue.main.asyncAfter(deadline: .now() + 2) { lastCopiedItemId = nil } }
70-77: Add loading state during channel closure.Consider adding a loading state to provide feedback during the channel closure process.
+@State private var isClosing = false Button("Close") { showingConfirmationAlert = true } .foregroundColor(.red) .padding() +.disabled(isClosing) // In the confirmation action: Button("Yes", role: .destructive) { + isClosing = true viewModel.close() refreshFlag = true if !showingChannelDetailViewErrorAlert { DispatchQueue.main.asyncAfter(deadline: .now() + 0.25) { + isClosing = false self.presentationMode.wrappedValue.dismiss() } } }LDKNodeMonday/View/Settings/SettingsView.swift (3)
31-48: Well-organized settings structure with minor style issues.The settings form is well-organized with clear sections. However, there are some comment style issues to address.
Fix comment spacing as per SwiftLint:
-// Wallet +// Wallet -// Lightning node +// Lightning node -// Danger zone +// Danger zone
52-69: Add visual feedback for node ID copy operation.Consider adding visual feedback when the node ID is copied to improve user experience.
+@State private var nodeIdCopied = false Button { UIPasteboard.general.string = viewModel.nodeID + nodeIdCopied = true + DispatchQueue.main.asyncAfter(deadline: .now() + 2) { + nodeIdCopied = false + } } label: { Image(systemName: nodeIdCopied ? "checkmark" : "doc.on.doc") .resizable() .scaledToFit() .frame(width: 24, height: 24) .foregroundColor(.accentColor) }
154-174: Enhance error recovery mechanism.Consider adding retry functionality for failed operations.
+@State private var retryCount = 0 +let maxRetries = 3 .onAppear { Task { + func retryOperation() async { + do { viewModel.getNodeID() viewModel.getNetwork() viewModel.getEsploraUrl() viewModel.getColor() await viewModel.getStatus() + retryCount = 0 + } catch { + if retryCount < maxRetries { + retryCount += 1 + try await Task.sleep(nanoseconds: UInt64(1_000_000_000 * retryCount)) + await retryOperation() + } + } + } + await retryOperation() } }LDKNodeMonday/View/Settings/Channel/ChannelAddView.swift (2)
12-20: LGTM! Well-structured view with proper state management.The view follows SwiftUI best practices with appropriate property wrappers and clear separation of concerns. Consider adding a documentation comment describing the view's purpose and responsibilities.
Add a documentation comment like:
+/// A view that allows users to add a new Lightning Network channel by providing +/// node ID, address, and channel capacity information. Supports manual input, +/// QR code scanning, and clipboard paste functionality. struct ChannelAddView: View {
217-237: Enhance QR code scanning error handling.The error handling could be more specific to help users understand and resolve scanning issues.
Consider enhancing the error handling:
func handleScan(result: Result<ScanResult, ScanError>) { isShowingScanner = false switch result { case .success(let result): let scannedQRCode = result.string.lowercased() if let peer = scannedQRCode.parseConnectionInfo() { viewModel.nodeId = peer.nodeID viewModel.address = peer.address } else { viewModel.channelAddViewError = .init( - title: "QR Parsing Error", - detail: "Failed to parse the QR code." + title: "Invalid QR Code", + detail: "The scanned QR code doesn't contain valid Lightning node connection information. Please ensure you're scanning a Lightning node QR code." ) } case .failure(let error): + let errorDetail: String + switch error { + case .badInput: + errorDetail = "The camera feed is not working properly." + case .badOutput: + errorDetail = "Failed to process the scanned image." + case .permissionDenied: + errorDetail = "Camera access is required to scan QR codes. Please enable it in Settings." + default: + errorDetail = error.localizedDescription + } viewModel.channelAddViewError = .init( title: "Scan Error", - detail: error.localizedDescription + detail: errorDetail ) } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (20)
LDKNodeMonday.xcodeproj/project.pbxproj(4 hunks)LDKNodeMonday/Service/Lightning Service/LightningNodeService.swift(1 hunks)LDKNodeMonday/Utilities/Constants.swift(1 hunks)LDKNodeMonday/View/Home/BitcoinView.swift(1 hunks)LDKNodeMonday/View/Home/NetworkSettingsView.swift(1 hunks)LDKNodeMonday/View/Profile/Channel/ChannelAddView.swift(0 hunks)LDKNodeMonday/View/Profile/Channel/ChannelsListView.swift(0 hunks)LDKNodeMonday/View/Profile/Danger/SeedView.swift(0 hunks)LDKNodeMonday/View/Profile/NodeIDView.swift(0 hunks)LDKNodeMonday/View/Profile/Peers/DisconnectView.swift(0 hunks)LDKNodeMonday/View/Profile/Peers/PeerView.swift(0 hunks)LDKNodeMonday/View/Profile/Peers/PeersListView.swift(0 hunks)LDKNodeMonday/View/Settings/Channel/ChannelAddView.swift(1 hunks)LDKNodeMonday/View/Settings/Channel/ChannelDetailView.swift(1 hunks)LDKNodeMonday/View/Settings/Channel/ChannelsListView.swift(1 hunks)LDKNodeMonday/View/Settings/Danger/SeedView.swift(1 hunks)LDKNodeMonday/View/Settings/Peers/DisconnectView.swift(1 hunks)LDKNodeMonday/View/Settings/Peers/PeerView.swift(1 hunks)LDKNodeMonday/View/Settings/Peers/PeersListView.swift(1 hunks)LDKNodeMonday/View/Settings/SettingsView.swift(1 hunks)
💤 Files with no reviewable changes (7)
- LDKNodeMonday/View/Profile/Peers/DisconnectView.swift
- LDKNodeMonday/View/Profile/Channel/ChannelsListView.swift
- LDKNodeMonday/View/Profile/Danger/SeedView.swift
- LDKNodeMonday/View/Profile/Peers/PeerView.swift
- LDKNodeMonday/View/Profile/NodeIDView.swift
- LDKNodeMonday/View/Profile/Peers/PeersListView.swift
- LDKNodeMonday/View/Profile/Channel/ChannelAddView.swift
✅ Files skipped from review due to trivial changes (2)
- LDKNodeMonday/Utilities/Constants.swift
- LDKNodeMonday/View/Home/NetworkSettingsView.swift
🧰 Additional context used
📓 Learnings (1)
LDKNodeMonday.xcodeproj/project.pbxproj (1)
Learnt from: danielnordh
PR: reez/Monday#131
File: LDKNodeMonday.xcodeproj/project.pbxproj:180-180
Timestamp: 2024-12-05T17:54:34.328Z
Learning: Multiple LDKNode framework references in `LDKNodeMonday.xcodeproj/project.pbxproj` may be intentional when using custom forks or multiple versions of LDKNode, and should not be flagged as duplicates.
🪛 SwiftLint (0.57.0)
LDKNodeMonday/View/Settings/Peers/DisconnectView.swift
[Warning] 25-25: Prefer at least one space after slashes for comments
(comment_spacing)
LDKNodeMonday/View/Settings/Channel/ChannelAddView.swift
[Warning] 51-51: Unused parameter in a closure should be replaced with _
(unused_closure_parameter)
[Warning] 72-72: Unused parameter in a closure should be replaced with _
(unused_closure_parameter)
LDKNodeMonday/View/Settings/Danger/SeedView.swift
[Warning] 67-67: Prefer at least one space after slashes for comments
(comment_spacing)
LDKNodeMonday/View/Settings/SettingsView.swift
[Warning] 96-96: Prefer at least one space after slashes for comments
(comment_spacing)
[Warning] 109-109: Prefer at least one space after slashes for comments
(comment_spacing)
[Warning] 125-125: Prefer at least one space after slashes for comments
(comment_spacing)
[Warning] 144-144: Prefer at least one space after slashes for comments
(comment_spacing)
LDKNodeMonday/View/Settings/Peers/PeersListView.swift
[Warning] 43-43: Using ternary to call Void functions should be avoided
(void_function_in_ternary)
LDKNodeMonday/View/Settings/Peers/PeerView.swift
[Warning] 50-50: Unused parameter in a closure should be replaced with _
(unused_closure_parameter)
[Warning] 71-71: Unused parameter in a closure should be replaced with _
(unused_closure_parameter)
🔇 Additional comments (6)
LDKNodeMonday/Service/Lightning Service/LightningNodeService.swift (1)
27-27: Verify the default Esplora URL configuration.
The default Esplora URL is hardcoded to use LQWD's signet server. This might need to be aligned with the network type to prevent cross-network issues.
Consider:
- Making the default URL network-aware using the
networkparameter - Moving the URL configuration to a configuration file for easier maintenance
- Adding a fallback mechanism if the LQWD server is unreachable
- ?? EsploraServer.lqwd_signet.url
+ ?? {
+ switch self.network {
+ case .signet:
+ return EsploraServer.lqwd_signet.url
+ case .testnet:
+ return EsploraServer.testnet.url
+ // Add other network cases
+ default:
+ return EsploraServer.default.url
+ }
+ }()LDKNodeMonday/View/Settings/Peers/DisconnectView.swift (1)
11-15: LGTM! Properties follow SwiftUI best practices
The view properties are well-structured using appropriate property wrappers.
LDKNodeMonday/View/Settings/Channel/ChannelsListView.swift (1)
15-89: LGTM! Well-structured list implementation.
The implementation follows SwiftUI best practices with proper empty state handling, clear sorting logic, and clean navigation structure.
LDKNodeMonday/View/Home/BitcoinView.swift (1)
242-242: LGTM! Clean replacement of NodeIDView with SettingsView.
The change aligns with the PR objectives of updating the Settings view structure.
LDKNodeMonday.xcodeproj/project.pbxproj (2)
461-474: LGTM! Clean project structure update.
The renaming of the Profile group to Settings and associated file reference updates are consistent with the PR objectives.
69-69: LGTM! Proper file reference updates.
The file reference changes for SettingsView.swift are correctly reflected in the project structure.
Also applies to: 150-150, 686-686
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
LDKNodeMonday/View/Settings/Peers/PeersListView.swift (1)
77-81:⚠️ Potential issueAdd error handling for async operations
The async tasks still lack error handling as mentioned in the previous review.
.onAppear { Task { - viewModel.listPeers() - viewModel.getColor() + do { + try await viewModel.listPeers() + try await viewModel.getColor() + } catch { + // Handle or propagate error appropriately + print("Error updating peers list: \(error)") + } } }
🧹 Nitpick comments (7)
LDKNodeMonday/View/Settings/Peers/PeersListView.swift (2)
42-53: Simplify connection status display logicThe current ternary implementation for connection status is complex and flagged by SwiftLint. Consider extracting this into a separate view component.
- peer.isConnected - ? HStack(alignment: .firstTextBaseline, spacing: 2) { - Text("Connected") - Image(systemName: "checkmark") - }.font(.caption) - .foregroundColor(.secondary) - : HStack { - Text("Not Connected") - Image(systemName: "xmark") - }.font(.caption) - .foregroundColor(.secondary) + ConnectionStatusView(isConnected: peer.isConnected) + struct ConnectionStatusView: View { + let isConnected: Bool + + var body: some View { + HStack(alignment: .firstTextBaseline, spacing: 2) { + Text(isConnected ? "Connected" : "Not Connected") + Image(systemName: isConnected ? "checkmark" : "xmark") + } + .font(.caption) + .foregroundColor(.secondary) + } + }🧰 Tools
🪛 SwiftLint (0.57.0)
[Warning] 43-43: Using ternary to call Void functions should be avoided
(void_function_in_ternary)
38-41: Enhance accessibility for node ID displayWhile the view implements dynamic type sizing, the truncated node ID could be made more accessible.
- Text("\(peer.nodeId) ") - .truncationMode(.middle) - .lineLimit(1) - .font(.subheadline.weight(.medium)) + Text("\(peer.nodeId) ") + .truncationMode(.middle) + .lineLimit(1) + .font(.subheadline.weight(.medium)) + .accessibilityLabel("Node ID: \(peer.nodeId)") + .accessibilityHint("Tap to view peer details")LDKNodeMonday/View/Settings/Peers/PeerDetailsView.swift (5)
2-2: Update file header comment to match the actual filenameThe file header shows "DisconnectView.swift" but the actual filename is "PeerDetailsView.swift".
-// DisconnectView.swift +// PeerDetailsView.swift
11-16: Consider renaming DisconnectViewModel to PeerDetailsViewModelThe current ViewModel name focuses on a single action (disconnect) while the view handles broader peer details functionality. A more appropriate name would better reflect its purpose.
- @ObservedObject var viewModel: DisconnectViewModel + @ObservedObject var viewModel: PeerDetailsViewModel
26-26: Remove commented codeRemove the commented frame modifier if it's no longer needed.
- //.frame(width: 100)🧰 Tools
🪛 SwiftLint (0.57.0)
[Warning] 26-26: Prefer at least one space after slashes for comments
(comment_spacing)
66-74: Enhance error alert implementationThe current error handling could be improved:
- Unknown is used as a fallback title
- Empty string is used as a fallback detail
- Error state might persist after dismissal
Consider using more meaningful fallback messages and ensure proper error state cleanup.
.alert(isPresented: $showErrorAlert) { Alert( - title: Text(viewModel.disconnectViewError?.title ?? "Unknown"), - message: Text(viewModel.disconnectViewError?.detail ?? ""), + title: Text(viewModel.disconnectViewError?.title ?? "Error"), + message: Text(viewModel.disconnectViewError?.detail ?? "An unexpected error occurred"), dismissButton: .default(Text("OK")) { viewModel.disconnectViewError = nil + showErrorAlert = false } ) }
88-96: Enhance preview with multiple statesConsider adding previews for different states like loading, error, and different node ID lengths to ensure the UI handles all cases correctly.
#if DEBUG #Preview { + NavigationView { PeerDetailsView( viewModel: .init( nodeId: "03e39c737a691931dac0f9f9ee803f2ab08f7fd3bbb25ec08d9b8fdb8f51d3a8db" ) ) + } + } + + #Preview("Error State") { + NavigationView { + PeerDetailsView( + viewModel: .init( + nodeId: "short_id", + initialError: .init(title: "Error", detail: "Sample error message") + ) + ) + } } #endif
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
LDKNodeMonday.xcodeproj/project.pbxproj(8 hunks)LDKNodeMonday/View/Settings/Peers/PeerDetailsView.swift(1 hunks)LDKNodeMonday/View/Settings/Peers/PeersListView.swift(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- LDKNodeMonday.xcodeproj/project.pbxproj
🧰 Additional context used
🪛 SwiftLint (0.57.0)
LDKNodeMonday/View/Settings/Peers/PeerDetailsView.swift
[Warning] 26-26: Prefer at least one space after slashes for comments
(comment_spacing)
LDKNodeMonday/View/Settings/Peers/PeersListView.swift
[Warning] 43-43: Using ternary to call Void functions should be avoided
(void_function_in_ternary)
🔇 Additional comments (1)
LDKNodeMonday/View/Settings/Peers/PeersListView.swift (1)
11-13: LGTM! Follows SwiftUI MVVM best practices
The view is properly structured with an observed view model for state management.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
LDKNodeMonday/View/Settings/Peers/PeerView.swift (1)
124-144: 🛠️ Refactor suggestionSimplify connection logic
The dismissal logic is redundant as it performs the same action regardless of the error alert state.
Button("Connect Peer") { self.viewModel.isProgressViewShowing = true Task { await viewModel.connect( nodeId: viewModel.nodeId, address: viewModel.address ) } - if showingPeerViewErrorAlert == true { - DispatchQueue.main.asyncAfter(deadline: .now() + 0.25) { - self.presentationMode.wrappedValue.dismiss() - } - } - - if showingPeerViewErrorAlert == false { - DispatchQueue.main.asyncAfter(deadline: .now() + 0.25) { - self.presentationMode.wrappedValue.dismiss() - } - } + DispatchQueue.main.asyncAfter(deadline: .now() + 0.25) { + self.presentationMode.wrappedValue.dismiss() + } }LDKNodeMonday/View/Settings/Peers/PeerDetailsView.swift (1)
57-62:⚠️ Potential issueImprove disconnect operation handling
The current implementation has several potential issues:
- No loading state during the disconnect operation
- Hardcoded delay for dismissal
- No error handling for the dismiss operation
Consider implementing a proper async/await pattern with loading state.
- viewModel.disconnect() - - DispatchQueue.main.asyncAfter(deadline: .now() + 0.25) { - self.presentationMode.wrappedValue.dismiss() - } + Task { + await viewModel.disconnect() + await MainActor.run { + self.presentationMode.wrappedValue.dismiss() + } + }
🧹 Nitpick comments (12)
LDKNodeMonday/View/Settings/Peers/PeerView.swift (4)
50-52: Replace unused parameter with underscoreThe closure parameter
oldValueis unused.-).onChange(of: viewModel.nodeId) { oldValue, newValue in +).onChange(of: viewModel.nodeId) { _, newValue in🧰 Tools
🪛 SwiftLint (0.57.0)
[Warning] 50-50: Unused parameter in a closure should be replaced with _
(unused_closure_parameter)
71-73: Replace unused parameter with underscoreThe closure parameter
oldValueis unused.-).onChange(of: viewModel.address) { oldValue, newValue in +).onChange(of: viewModel.address) { _, newValue in🧰 Tools
🪛 SwiftLint (0.57.0)
[Warning] 71-71: Unused parameter in a closure should be replaced with _
(unused_closure_parameter)
187-216: Consider moving validation logic to ViewModelThe QR code validation logic could be moved to the ViewModel to maintain better separation of concerns and improve testability.
+// In PeerViewModel +func validatePeer(_ peer: PeerInfo) -> Bool { + return peer.nodeID.count >= 32 && peer.address.contains(":") +} + +func handleInvalidPeer() -> PeerViewError { + return .init( + title: "Invalid Format", + detail: "The scanned QR code contains invalid peer information." + ) +} // In PeerView func handleScan(result: Result<ScanResult, ScanError>) { isShowingScanner = false switch result { case .success(let result): let scannedQRCode = result.string.lowercased() if let peer = scannedQRCode.parseConnectionInfo() { - guard peer.nodeID.count >= 32, - peer.address.contains(":") - else { - self.viewModel.peerViewError = .init( - title: "Invalid Format", - detail: "The scanned QR code contains invalid peer information." - ) + guard viewModel.validatePeer(peer) else { + self.viewModel.peerViewError = viewModel.handleInvalidPeer() return } viewModel.nodeId = peer.nodeID viewModel.address = peer.address
220-224: Enhance preview with multiple statesConsider adding more preview states to help with UI development and testing.
#if DEBUG #Preview { - PeerView(viewModel: .init()) + Group { + PeerView(viewModel: .init()) + .previewDisplayName("Default State") + + PeerView(viewModel: { + let vm = PeerViewModel() + vm.nodeId = "03a5b467d7f...4c2b099b8250c" + vm.address = "172.18.0.2:9735" + return vm + }()) + .previewDisplayName("Filled State") + + PeerView(viewModel: { + let vm = PeerViewModel() + vm.isProgressViewShowing = true + return vm + }()) + .previewDisplayName("Loading State") + } } #endifLDKNodeMonday/View/Settings/Danger/SeedView.swift (5)
12-19: Consider adding memory security measures for sensitive dataSince this view handles sensitive recovery phrases, consider implementing additional security measures:
- Mark sensitive variables with
@SecureField- Clear sensitive data when the view disappears
struct SeedView: View { @ObservedObject var viewModel: SeedViewModel @State private var showAlert = false @State private var showRecoveryPhrase = false @State private var isCopied = false @State private var showCheckmark = false @State private var showingSeedViewErrorAlert = false + + private func clearSensitiveData() { + showRecoveryPhrase = false + isCopied = false + UIPasteboard.general.string = "" + }
24-43: Enhance accessibility for warning messageThe warning message should be more accessible:
- Add
accessibilityLabelfor better screen reader support- Consider using semantic
.alertroleText( "Warning! \n\n Never share the recovery phrase. Doing so will put your funds at risk." ) .font(.body) .multilineTextAlignment(.center) .padding(40) +.accessibilityLabel("Security Warning") +.accessibilityAddTraits(.isAlert)
86-94: Enhance error handling and user feedbackConsider improving the error handling:
- Add specific error cases instead of using optional strings
- Provide more detailed error messages
- Add retry functionality where appropriate
.alert(isPresented: $showingSeedViewErrorAlert) { Alert( - title: Text(viewModel.seedViewError?.title ?? "Unknown"), - message: Text(viewModel.seedViewError?.detail ?? ""), + title: Text(viewModel.seedViewError?.title ?? "Error"), + message: Text(viewModel.seedViewError?.detail ?? "An unexpected error occurred. Please try again."), dismissButton: .default(Text("OK")) { viewModel.seedViewError = nil + // Consider adding retry logic here if appropriate } ) }
99-103: Enhance preview with different statesConsider adding multiple preview states to help with UI development:
- Error state
- Loading state
- Different phrase lengths (12/24 words)
#if DEBUG struct SeedView_Previews: PreviewProvider { static var previews: some View { Group { SeedView(viewModel: .init()) // Default state SeedView(viewModel: { let vm = SeedViewModel() vm.seedViewError = .init(title: "Error", detail: "Test error") return vm }()) .previewDisplayName("Error State") // Add more preview cases as needed } } } #endif
74-74: Fix comment spacingAdd a space after the double slash for consistency with Swift style guidelines.
- //Spacer() + // Spacer()🧰 Tools
🪛 SwiftLint (0.57.0)
[Warning] 74-74: Prefer at least one space after slashes for comments
(comment_spacing)
LDKNodeMonday/View/Settings/Peers/PeerDetailsView.swift (3)
2-2: Update file header to match new filenameThe file header still shows the old filename "DisconnectView.swift". Update it to "PeerDetailsView.swift" for consistency.
-// DisconnectView.swift +// PeerDetailsView.swift
25-25: Remove commented-out codeRemove the commented frame modifier as it adds noise to the codebase.
- //.frame(width: 100)🧰 Tools
🪛 SwiftLint (0.57.0)
[Warning] 25-25: Prefer at least one space after slashes for comments
(comment_spacing)
20-36: Consider simplifying the List structureThe current implementation uses a List with a single VStack item. Consider simplifying to just use the VStack directly if there won't be multiple items.
- List { VStack(alignment: .leading) { Text("Node ID") .font(.subheadline.weight(.medium)) Text(viewModel.nodeId.description) .truncationMode(.middle) .lineLimit(1) .font(.caption) .foregroundColor(.secondary) } .padding(.top, 5) - .listRowSeparator(.hidden) - .listRowBackground(Color.clear) - } - .listStyle(.plain) + .padding(.horizontal)🧰 Tools
🪛 SwiftLint (0.57.0)
[Warning] 25-25: Prefer at least one space after slashes for comments
(comment_spacing)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
LDKNodeMonday/View/Settings/Danger/SeedView.swift(1 hunks)LDKNodeMonday/View/Settings/Peers/PeerDetailsView.swift(1 hunks)LDKNodeMonday/View/Settings/Peers/PeerView.swift(1 hunks)
🧰 Additional context used
🪛 SwiftLint (0.57.0)
LDKNodeMonday/View/Settings/Danger/SeedView.swift
[Warning] 74-74: Prefer at least one space after slashes for comments
(comment_spacing)
LDKNodeMonday/View/Settings/Peers/PeerDetailsView.swift
[Warning] 25-25: Prefer at least one space after slashes for comments
(comment_spacing)
LDKNodeMonday/View/Settings/Peers/PeerView.swift
[Warning] 50-50: Unused parameter in a closure should be replaced with _
(unused_closure_parameter)
[Warning] 71-71: Unused parameter in a closure should be replaced with _
(unused_closure_parameter)
🔇 Additional comments (6)
LDKNodeMonday/View/Settings/Peers/PeerView.swift (2)
1-11: LGTM: Clean imports and structure
The file structure is clean and imports are appropriate for the required functionality.
12-19: LGTM: Well-structured state management
The view's state management follows SwiftUI best practices with appropriate use of property wrappers.
LDKNodeMonday/View/Settings/Danger/SeedView.swift (1)
51-76: LGTM: Clipboard security measures implemented
The implementation includes proper clipboard security measures:
- Clears clipboard after 60 seconds
- Provides visual feedback
- Resets UI state appropriately
🧰 Tools
🪛 SwiftLint (0.57.0)
[Warning] 74-74: Prefer at least one space after slashes for comments
(comment_spacing)
LDKNodeMonday/View/Settings/Peers/PeerDetailsView.swift (3)
10-15: LGTM! Properties are well-structured
The view properties follow SwiftUI best practices with appropriate use of property wrappers.
65-78: LGTM! Well-implemented error handling
The alert implementation follows best practices with:
- Proper error handling
- User-friendly error messages
- Fallback values for unknown errors
- Clean error state management
87-95: LGTM! Well-structured preview
The preview implementation follows SwiftUI best practices and provides a good sample node ID for development testing.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (10)
LDKNodeMonday/View/Settings/Channel/ChannelDetailView.swift (2)
38-55: Consider extracting magic numbersThe copy feedback implementation is good, but consider extracting the hardcoded 2-second duration to a named constant for better maintainability.
+ private let copyFeedbackDuration: TimeInterval = 2.0 Button { UIPasteboard.general.string = property.value lastCopiedItemId = property.name - DispatchQueue.main.asyncAfter(deadline: .now() + 2) { + DispatchQueue.main.asyncAfter(deadline: .now() + copyFeedbackDuration) { lastCopiedItemId = nil } }
Line range hint
78-107: Consider extracting dismissal delay constantThe alert handling is well-implemented, but consider extracting the hardcoded 0.25-second dismissal delay to a named constant for consistency and maintainability.
+ private let dismissalDelay: TimeInterval = 0.25 Button("Yes", role: .destructive) { viewModel.close() refreshFlag = true if !showingChannelDetailViewErrorAlert { - DispatchQueue.main.asyncAfter(deadline: .now() + 0.25) { + DispatchQueue.main.asyncAfter(deadline: .now() + dismissalDelay) { self.presentationMode.wrappedValue.dismiss() } } }LDKNodeMonday/View/Settings/SettingsView.swift (5)
27-27: Remove unused state variable.The
showGreetingstate variable appears to be unused in the view.- @State private var showGreeting = true
15-25: Consider grouping related state variables.Consider organizing state variables into logical groups for better maintainability:
- @State private var isCopied = false - @State private var showCheckmark = false - @State private var showingNodeIDErrorAlert = false - @State private var isSeedPresented = false - @State private var showingStopNodeConfirmation = false - @State private var showingDeleteSeedConfirmation = false - @State private var showingShowSeedConfirmation = false - @State private var showingResetAppConfirmation = false - @State private var isViewPeersPresented = false - @State private var refreshFlag = false - @State private var isPaymentsPresented = false + // UI State + @State private var isCopied = false + @State private var showCheckmark = false + @State private var refreshFlag = false + + // Navigation State + @State private var isSeedPresented = false + @State private var isViewPeersPresented = false + @State private var isPaymentsPresented = false + + // Alert State + @State private var showingNodeIDErrorAlert = false + @State private var showingStopNodeConfirmation = false + @State private var showingDeleteSeedConfirmation = false + @State private var showingShowSeedConfirmation = false + @State private var showingResetAppConfirmation = false
60-69: Add visual feedback for copy action.Consider adding visual feedback when the node ID is copied to improve user experience.
Button { UIPasteboard.general.string = viewModel.nodeID + withAnimation { + isCopied = true + DispatchQueue.main.asyncAfter(deadline: .now() + 2) { + isCopied = false + } + } } label: { - Image(systemName: "doc.on.doc") + Image(systemName: isCopied ? "checkmark" : "doc.on.doc") .resizable() .scaledToFit() .frame(width: 24, height: 24) .foregroundColor(.accentColor) }
144-144: Remove or implement commented code.Either remove the commented out scrollContentBackground modifier or implement it if needed.
- //.scrollContentBackground(.hidden) // uncomment if we want white background🧰 Tools
🪛 SwiftLint (0.57.0)
[Warning] 144-144: Prefer at least one space after slashes for comments
(comment_spacing)
154-161: Add error handling for async operations.Consider adding error handling for the async operations in onAppear.
.onAppear { Task { + do { viewModel.getNodeID() viewModel.getNetwork() viewModel.getEsploraUrl() - await viewModel.getStatus() + try await viewModel.getStatus() + } catch { + viewModel.nodeIDError = .init( + title: "Error", + detail: "Failed to fetch node status: \(error.localizedDescription)" + ) + } } }LDKNodeMonday/View/Settings/Peers/PeerDetailsView.swift (2)
25-25: Remove commented codeThe commented frame modifier serves no purpose and should be removed.
- //.frame(width: 100)🧰 Tools
🪛 SwiftLint (0.57.0)
[Warning] 25-25: Prefer at least one space after slashes for comments
(comment_spacing)
66-79: Consider consolidating error alert logicThe error alert setup is split between the alert view modifier and onReceive. Consider combining them into a single error handling view extension.
extension View { func errorAlert(error: Binding<MondayError?>) -> some View { let showAlert = Binding( get: { error.wrappedValue != nil }, set: { if !$0 { error.wrappedValue = nil } } ) return alert(isPresented: showAlert) { Alert( title: Text(error.wrappedValue?.title ?? "Unknown"), message: Text(error.wrappedValue?.detail ?? ""), dismissButton: .default(Text("OK")) ) } } }Usage:
- .alert(isPresented: $showErrorAlert) { - Alert( - title: Text(viewModel.disconnectViewError?.title ?? "Unknown"), - message: Text(viewModel.disconnectViewError?.detail ?? ""), - dismissButton: .default(Text("OK")) { - viewModel.disconnectViewError = nil - } - ) - } - .onReceive(viewModel.$disconnectViewError) { errorMessage in - if errorMessage != nil { - showErrorAlert = true - } - } + .errorAlert(error: $viewModel.disconnectViewError)LDKNodeMonday/View/Settings/Peers/PeersListView.swift (1)
42-53: Refactor connection status view for better readabilityThe ternary operator for view construction makes the code harder to read. Consider extracting the connection status view into a separate view component.
struct PeerConnectionStatusView: View { let isConnected: Bool var body: some View { HStack { Text(isConnected ? "Connected" : "Not Connected") Image(systemName: isConnected ? "checkmark" : "xmark") } .font(.caption) .foregroundColor(.secondary) } }Usage:
- peer.isConnected - ? HStack(alignment: .firstTextBaseline, spacing: 2) { - Text("Connected") - Image(systemName: "checkmark") - }.font(.caption) - .foregroundColor(.secondary) - : HStack { - Text("Not Connected") - Image(systemName: "xmark") - }.font(.caption) - .foregroundColor(.secondary) + PeerConnectionStatusView(isConnected: peer.isConnected)🧰 Tools
🪛 SwiftLint (0.57.0)
[Warning] 43-43: Using ternary to call Void functions should be avoided
(void_function_in_ternary)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
LDKNodeMonday/View Model/Profile/Channel/ChannelAddViewModel.swift(0 hunks)LDKNodeMonday/View Model/Profile/Channel/ChannelDetailViewModel.swift(0 hunks)LDKNodeMonday/View Model/Profile/Channel/ChannelsListViewModel.swift(0 hunks)LDKNodeMonday/View Model/Profile/Danger/SeedViewModel.swift(0 hunks)LDKNodeMonday/View Model/Profile/NodeIDViewModel.swift(0 hunks)LDKNodeMonday/View Model/Profile/Peer/DisconnectViewModel.swift(1 hunks)LDKNodeMonday/View Model/Profile/Peer/PeerViewModel.swift(0 hunks)LDKNodeMonday/View Model/Profile/Peer/PeersListViewModel.swift(0 hunks)LDKNodeMonday/View/Settings/Channel/ChannelAddView.swift(1 hunks)LDKNodeMonday/View/Settings/Channel/ChannelDetailView.swift(1 hunks)LDKNodeMonday/View/Settings/Channel/ChannelsListView.swift(1 hunks)LDKNodeMonday/View/Settings/Peers/PeerDetailsView.swift(1 hunks)LDKNodeMonday/View/Settings/Peers/PeerView.swift(1 hunks)LDKNodeMonday/View/Settings/Peers/PeersListView.swift(1 hunks)LDKNodeMonday/View/Settings/SettingsView.swift(1 hunks)
💤 Files with no reviewable changes (7)
- LDKNodeMonday/View Model/Profile/Channel/ChannelDetailViewModel.swift
- LDKNodeMonday/View Model/Profile/Danger/SeedViewModel.swift
- LDKNodeMonday/View Model/Profile/Peer/PeersListViewModel.swift
- LDKNodeMonday/View Model/Profile/Channel/ChannelAddViewModel.swift
- LDKNodeMonday/View Model/Profile/Peer/PeerViewModel.swift
- LDKNodeMonday/View Model/Profile/Channel/ChannelsListViewModel.swift
- LDKNodeMonday/View Model/Profile/NodeIDViewModel.swift
🚧 Files skipped from review as they are similar to previous changes (3)
- LDKNodeMonday/View/Settings/Channel/ChannelsListView.swift
- LDKNodeMonday/View/Settings/Peers/PeerView.swift
- LDKNodeMonday/View/Settings/Channel/ChannelAddView.swift
🧰 Additional context used
📓 Learnings (1)
LDKNodeMonday/View/Settings/SettingsView.swift (1)
Learnt from: danielnordh
PR: reez/Monday#143
File: LDKNodeMonday/View/Settings/Danger/SeedView.swift:44-49
Timestamp: 2024-12-18T16:56:17.814Z
Learning: In SwiftUI for the LDKNodeMonday project, additional BIP39 validation for the seed mnemonic isn't strictly necessary because the node won't start unless the seed is valid.
🪛 SwiftLint (0.57.0)
LDKNodeMonday/View/Settings/Peers/PeerDetailsView.swift
[Warning] 25-25: Prefer at least one space after slashes for comments
(comment_spacing)
LDKNodeMonday/View/Settings/Peers/PeersListView.swift
[Warning] 43-43: Using ternary to call Void functions should be avoided
(void_function_in_ternary)
LDKNodeMonday/View/Settings/SettingsView.swift
[Warning] 96-96: Prefer at least one space after slashes for comments
(comment_spacing)
[Warning] 109-109: Prefer at least one space after slashes for comments
(comment_spacing)
[Warning] 125-125: Prefer at least one space after slashes for comments
(comment_spacing)
[Warning] 144-144: Prefer at least one space after slashes for comments
(comment_spacing)
🔇 Additional comments (7)
LDKNodeMonday/View/Settings/Channel/ChannelDetailView.swift (3)
24-63: LGTM: Improved layout structure
The restructuring from ZStack to VStack with a plain List improves the view hierarchy and follows SwiftUI best practices. The layout is cleaner and more maintainable.
28-36: LGTM: Well-structured text hierarchy
The text styling creates a clear visual hierarchy with:
- Medium subheadline for property names
- Secondary-colored caption for values
- Appropriate truncation for long values
67-77: LGTM: Well-configured navigation and accessibility
The navigation setup includes:
- Appropriate dynamic type size limits for accessibility
- Well-placed close button with proper destructive styling
- Clear navigation title configuration
LDKNodeMonday/View/Settings/SettingsView.swift (1)
91-141: Enhance safety for critical actions.
Consider adding additional safeguards for critical actions in the danger zone.
Also, fix the comment spacing issues in the following lines:
- Text("Stop Node") //, systemImage: "exclamationmark.octagon")
+ Text("Stop Node") // , systemImage: "exclamationmark.octagon")
- Text("Reset Preferences") //, systemImage: "minus.diamond")
+ Text("Reset Preferences") // , systemImage: "minus.diamond")
- Text("Delete Seed") //, systemImage: "delete.left")
+ Text("Delete Seed") // , systemImage: "delete.left")🧰 Tools
🪛 SwiftLint (0.57.0)
[Warning] 96-96: Prefer at least one space after slashes for comments
(comment_spacing)
[Warning] 109-109: Prefer at least one space after slashes for comments
(comment_spacing)
[Warning] 125-125: Prefer at least one space after slashes for comments
(comment_spacing)
LDKNodeMonday/View Model/Profile/Peer/DisconnectViewModel.swift (1)
Line range hint 19-35: LGTM! Well-structured async implementation
The async implementation properly:
- Maintains error handling on the main thread
- Distinguishes between NodeError and unexpected errors
- Provides clear error messages to the UI
LDKNodeMonday/View/Settings/Peers/PeerDetailsView.swift (1)
57-62: LGTM! Well-implemented async disconnect flow
The implementation properly:
- Uses structured concurrency with Task
- Ensures UI updates on MainActor
- Handles view dismissal after disconnect
LDKNodeMonday/View/Settings/Peers/PeersListView.swift (1)
77-79:
Add error handling for listPeers
The listPeers() call should be marked as async and include error handling.
.onAppear {
Task {
- viewModel.listPeers()
+ do {
+ try await viewModel.listPeers()
+ } catch {
+ // Handle error appropriately
+ print("Failed to list peers: \(error)")
+ }
}
}Likely invalid or redundant comment.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
LDKNodeMonday/View/Settings/SettingsView.swift (1)
94-144:⚠️ Potential issueEnhance safety measures for critical actions.
While basic confirmation dialogs are implemented, critical actions like "Delete Seed" should have stronger safeguards.
Consider implementing a more robust confirmation mechanism:
+ struct DeleteConfirmationView: View { + let title: String + let action: () -> Void + @Binding var isPresented: Bool + @State private var confirmText = "" + + var body: some View { + NavigationView { + Form { + Section { + TextField("Type DELETE to confirm", text: $confirmText) + } footer: { + Text("This action cannot be undone") + } + } + .navigationTitle(title) + .toolbar { + ToolbarItem(placement: .confirmationAction) { + Button("Confirm") { + if confirmText.uppercased() == "DELETE" { + action() + isPresented = false + } + } + .disabled(confirmText.uppercased() != "DELETE") + } + ToolbarItem(placement: .cancellationAction) { + Button("Cancel") { + isPresented = false + } + } + } + } + } + }🧰 Tools
🪛 SwiftLint (0.57.0)
[Warning] 99-99: Prefer at least one space after slashes for comments
(comment_spacing)
[Warning] 112-112: Prefer at least one space after slashes for comments
(comment_spacing)
[Warning] 128-128: Prefer at least one space after slashes for comments
(comment_spacing)
🧹 Nitpick comments (8)
LDKNodeMonday/View/Settings/Peers/PeersListView.swift (3)
42-53: Simplify connection status view logic.The current ternary implementation is complex and flagged by SwiftLint. Consider extracting this into a separate view component for better readability and maintenance.
- peer.isConnected - ? HStack(alignment: .firstTextBaseline, spacing: 2) { - Text("Connected") - Image(systemName: "checkmark") - }.font(.caption) - .foregroundColor(.secondary) - : HStack { - Text("Not Connected") - Image(systemName: "xmark") - }.font(.caption) - .foregroundColor(.secondary) + ConnectionStatusView(isConnected: peer.isConnected)Add this new view:
struct ConnectionStatusView: View { let isConnected: Bool var body: some View { HStack(alignment: .firstTextBaseline, spacing: 2) { Text(isConnected ? "Connected" : "Not Connected") Image(systemName: isConnected ? "checkmark" : "xmark") } .font(.caption) .foregroundColor(.secondary) } }🧰 Tools
🪛 SwiftLint (0.57.0)
[Warning] 43-43: Using ternary to call Void functions should be avoided
(void_function_in_ternary)
38-41: Consider adding tooltip for full nodeId.Since the nodeId is truncated, consider adding a tooltip to show the full value on hover/long press.
Text("\(peer.nodeId) ") .truncationMode(.middle) .lineLimit(1) .font(.subheadline.weight(.medium)) + .help(peer.nodeId) // Shows full nodeId on hover
86-90: Enhance preview with multiple states.Consider adding previews for different states (empty list, multiple peers, connected/disconnected states) to help during development.
#if DEBUG - #Preview { - PeersListView(viewModel: .init()) + #Preview("Empty State") { + PeersListView(viewModel: .init()) + } + + #Preview("With Peers") { + let vm = PeersListViewModel() + vm.peers = [ + // Add sample peer data + ] + return PeersListView(viewModel: vm) } #endifLDKNodeMonday/View Model/Profile/SettingsViewModel.swift (1)
Line range hint
27-144: Consider consolidating error handling.The error handling pattern is duplicated across multiple methods (
stop,delete,onboarding,getNetwork,getEsploraUrl). Consider extracting the common error handling logic into a reusable method.+ private func handleError(_ operation: () throws -> Void) { + do { + try operation() + } catch let error as NodeError { + let errorString = handleNodeError(error) + DispatchQueue.main.async { + self.nodeIDError = .init(title: errorString.title, detail: errorString.detail) + } + } catch { + DispatchQueue.main.async { + self.nodeIDError = .init( + title: "Unexpected error", + detail: error.localizedDescription + ) + } + } + } func stop() { - do { - try LightningNodeService.shared.stop() - } catch let error as NodeError { - let errorString = handleNodeError(error) - DispatchQueue.main.async { - self.nodeIDError = .init(title: errorString.title, detail: errorString.detail) - } - } catch { - DispatchQueue.main.async { - self.nodeIDError = .init( - title: "Unexpected error", - detail: error.localizedDescription - ) - } - } + handleError { try LightningNodeService.shared.stop() } }LDKNodeMonday/View/Settings/SettingsView.swift (1)
99-99: Fix comment formatting.Add space after slashes in comments.
- Text("Stop Node") //, systemImage: "exclamationmark.octagon") + Text("Stop Node") // , systemImage: "exclamationmark.octagon")Also applies to: 112-112, 128-128
🧰 Tools
🪛 SwiftLint (0.57.0)
[Warning] 99-99: Prefer at least one space after slashes for comments
(comment_spacing)
LDKNodeMonday/View/Settings/Channel/ChannelDetailView.swift (3)
27-35: Consider adding accessibility labels for better VoiceOver support.While the text styling is well-implemented, consider enhancing accessibility by adding explicit accessibility labels that combine the property name and value.
VStack(alignment: .leading) { Text(property.name) .font(.subheadline.weight(.medium)) Text(property.value) .font(.caption) .foregroundColor(.secondary) .truncationMode(.middle) - .lineLimit(1) + .lineLimit(1) + .accessibilityLabel("\(property.name): \(property.value)") }
38-61: Optimize animation implementation.The current animation wrapper placement might cause unnecessary re-renders. Consider moving it to wrap only the changing content.
Button { UIPasteboard.general.string = property.value lastCopiedItemId = property.name DispatchQueue.main.asyncAfter(deadline: .now() + 2) { lastCopiedItemId = nil } } label: { HStack { - withAnimation { - Image( - systemName: lastCopiedItemId == property.name - ? "checkmark" : "doc.on.doc" - ) + Image( + systemName: lastCopiedItemId == property.name + ? "checkmark" : "doc.on.doc" + ) + .animation(.default, value: lastCopiedItemId) } }
73-83: Consider standardizing button padding.The padding on the close button might be excessive and inconsistent with iOS standards.
Button("Close") { showingConfirmationAlert = true } .foregroundColor(.red) -.padding() +.padding(.horizontal)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
LDKNodeMonday.xcodeproj/project.pbxproj(12 hunks)LDKNodeMonday/View Model/Profile/SettingsViewModel.swift(1 hunks)LDKNodeMonday/View/Settings/Channel/ChannelDetailView.swift(1 hunks)LDKNodeMonday/View/Settings/Channel/ChannelsListView.swift(1 hunks)LDKNodeMonday/View/Settings/Peers/PeersListView.swift(1 hunks)LDKNodeMonday/View/Settings/SettingsView.swift(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- LDKNodeMonday/View/Settings/Channel/ChannelsListView.swift
- LDKNodeMonday.xcodeproj/project.pbxproj
🧰 Additional context used
📓 Learnings (1)
LDKNodeMonday/View/Settings/SettingsView.swift (1)
Learnt from: danielnordh
PR: reez/Monday#143
File: LDKNodeMonday/View/Settings/Danger/SeedView.swift:44-49
Timestamp: 2024-12-18T16:56:17.814Z
Learning: In SwiftUI for the LDKNodeMonday project, additional BIP39 validation for the seed mnemonic isn't strictly necessary because the node won't start unless the seed is valid.
🪛 SwiftLint (0.57.0)
LDKNodeMonday/View/Settings/Peers/PeersListView.swift
[Warning] 43-43: Using ternary to call Void functions should be avoided
(void_function_in_ternary)
LDKNodeMonday/View/Settings/SettingsView.swift
[Warning] 99-99: Prefer at least one space after slashes for comments
(comment_spacing)
[Warning] 112-112: Prefer at least one space after slashes for comments
(comment_spacing)
[Warning] 128-128: Prefer at least one space after slashes for comments
(comment_spacing)
[Warning] 147-147: Prefer at least one space after slashes for comments
(comment_spacing)
🔇 Additional comments (6)
LDKNodeMonday/View/Settings/Peers/PeersListView.swift (3)
1-10: LGTM! File structure and imports are appropriate.
77-81: Add error handling for async task.
The async task should handle potential errors to prevent silent failures.
.onAppear {
Task {
- viewModel.listPeers()
+ do {
+ try await viewModel.listPeers()
+ } catch {
+ print("Error listing peers: \(error)")
+ // Consider showing error to user
+ }
}
}11-13: Verify if @StateObject would be more appropriate than @ObservedObject.
Consider using @StateObject instead of @ObservedObject if this view is responsible for the lifecycle of the view model. @StateObject ensures the view model survives view updates.
✅ Verification successful
Let me check the implementation details to understand how PeersListViewModel is created and used.
Keep @ObservedObject as it's correctly used for externally created view model
The current usage of @ObservedObject is appropriate since the view model is created and injected from parent views (SettingsView creates it via .init()). @StateObject would be incorrect here as it's meant for views that own their view model's lifecycle.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other usages of PeersListViewModel to understand ownership
rg -l "PeersListViewModel" --type swift
Length of output: 156
Script:
#!/bin/bash
# Check PeersListViewModel implementation and initialization
rg "PeersListViewModel" -A 5 --type swift
# Also check where PeersListView is used
rg "PeersListView" -A 5 --type swift
Length of output: 4272
LDKNodeMonday/View Model/Profile/SettingsViewModel.swift (1)
12-12: LGTM! Class renaming aligns with new Settings paradigm.
The renaming from NodeIDViewModel to SettingsViewModel better reflects its purpose and responsibilities. The removal of networkColor property simplifies state management.
LDKNodeMonday/View/Settings/SettingsView.swift (1)
145-145: LGTM! Good accessibility consideration.
The use of dynamicTypeSize with an upper limit ensures text remains readable while preventing layout issues.
LDKNodeMonday/View/Settings/Channel/ChannelDetailView.swift (1)
23-25: LGTM! Clean view hierarchy restructuring.
The switch to VStack with a direct List implementation provides a cleaner and more maintainable structure.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
LDKNodeMonday/View/Settings/SettingsView.swift (5)
12-27: Remove unused state variable.The
showGreetingstate variable on line 26 is declared but never used in the view.- @State private var showGreeting = true
43-49: Move URL manipulation logic to ViewModel.The URL string manipulation logic should be moved to the ViewModel to maintain separation of concerns and improve reusability.
- Label("Server", systemImage: "server.rack") - .badge(viewModel.esploraURL?.replacingOccurrences(of: "https://", with: "") - .replacingOccurrences( - of: "http://", - with: "" - ) ?? "No server") + Label("Server", systemImage: "server.rack") + .badge(viewModel.formattedEsploraURL)Add this computed property to your ViewModel:
var formattedEsploraURL: String { guard let url = esploraURL else { return "No server" } return url.replacingOccurrences(of: "https://", with: "") .replacingOccurrences(of: "http://", with: "") }
43-43: Track technical debt: NetworkSettingsView dependency.The comment indicates a dependency issue preventing the network settings from being moved to a dedicated view.
Would you like me to create a GitHub issue to track this technical debt?
67-79: Add accessibility label to copy button.The copy button should have an accessibility label to improve VoiceOver support.
Button { UIPasteboard.general.string = viewModel.nodeID showCheckmark = true DispatchQueue.main.asyncAfter(deadline: .now() + 2) { showCheckmark = false } } label: { Image(systemName: showCheckmark ? "checkmark" : "doc.on.doc") .resizable() .scaledToFit() .frame(width: 20, height: 20) .foregroundColor(showCheckmark ? .secondary : .accentColor) + .accessibilityLabel(showCheckmark ? "Copied" : "Copy node ID") }
190-194: Enhance preview with different states.Consider adding multiple preview states to help visualize different scenarios.
#if DEBUG #Preview("Default") { SettingsView(viewModel: .init()) } #Preview("With Error") { let viewModel = SettingsViewModel() viewModel.nodeIDError = .init(title: "Error", detail: "Something went wrong") return SettingsView(viewModel: viewModel) } #Preview("Node Running") { let viewModel = SettingsViewModel() // Set necessary state for running node return SettingsView(viewModel: viewModel) } #endif
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
LDKNodeMonday/View/Settings/SettingsView.swift(1 hunks)
🧰 Additional context used
📓 Learnings (1)
LDKNodeMonday/View/Settings/SettingsView.swift (1)
Learnt from: danielnordh
PR: reez/Monday#143
File: LDKNodeMonday/View/Settings/Danger/SeedView.swift:44-49
Timestamp: 2024-12-18T16:56:17.814Z
Learning: In SwiftUI for the LDKNodeMonday project, additional BIP39 validation for the seed mnemonic isn't strictly necessary because the node won't start unless the seed is valid.
🪛 SwiftLint (0.57.0)
LDKNodeMonday/View/Settings/SettingsView.swift
[Warning] 107-107: Prefer at least one space after slashes for comments
(comment_spacing)
[Warning] 120-120: Prefer at least one space after slashes for comments
(comment_spacing)
[Warning] 136-136: Prefer at least one space after slashes for comments
(comment_spacing)
[Warning] 155-155: Prefer at least one space after slashes for comments
(comment_spacing)
🔇 Additional comments (3)
LDKNodeMonday/View/Settings/SettingsView.swift (3)
1-11: LGTM!
The imports are appropriate for the functionality required by this view.
102-152: Enhance safety measures for critical actions.
The current implementation uses simple confirmation alerts for dangerous actions. Consider implementing stronger safeguards as previously suggested.
🧰 Tools
🪛 SwiftLint (0.57.0)
[Warning] 107-107: Prefer at least one space after slashes for comments
(comment_spacing)
[Warning] 120-120: Prefer at least one space after slashes for comments
(comment_spacing)
[Warning] 136-136: Prefer at least one space after slashes for comments
(comment_spacing)
165-172: Add error handling for async operations.
The async operations in onAppear lack proper error handling.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
LDKNodeMonday/View/Settings/SettingsView.swift (2)
99-152: 🛠️ Refactor suggestionEnhance safety for critical actions in the danger zone.
Consider adding stronger safeguards for critical actions by requiring explicit confirmation.
struct DeleteConfirmationView: View { @Binding var isPresented: Bool @State private var confirmText = "" let actionType: String let onConfirm: () -> Void var body: some View { VStack { Text("Type '\(actionType)' to confirm") .font(.headline) TextField("Confirmation", text: $confirmText) .textFieldStyle(RoundedBorderTextFieldStyle()) HStack { Button("Cancel") { isPresented = false } Button("Confirm") { if confirmText.uppercased() == actionType.uppercased() { onConfirm() isPresented = false } } .disabled(confirmText.uppercased() != actionType.uppercased()) .foregroundColor(.red) } } .padding() } }
170-175: 🛠️ Refactor suggestionConsider comprehensive error handling for async operations.
The
onAppearblock contains multiple async operations that should be properly handled for errors.Task { + do { - viewModel.getNodeID() - viewModel.getNetwork() - viewModel.getEsploraUrl() - await viewModel.getStatus() + try await viewModel.getNodeID() + try await viewModel.getNetwork() + try await viewModel.getEsploraUrl() + try await viewModel.getStatus() + } catch { + viewModel.nodeIDError = .init( + title: "Initialization Error", + detail: error.localizedDescription + ) + } }
🧹 Nitpick comments (4)
LDKNodeMonday/View/Settings/SettingsView.swift (4)
21-21: Remove unused state variable.The
showGreetingstate variable appears to be unused in the view.- @State private var showGreeting = true
40-46: Move URL formatting logic to ViewModel.Consider moving the URL formatting logic to the ViewModel to maintain separation of concerns and improve reusability.
// In SettingsViewModel + var formattedServerURL: String { + esploraURL? + .replacingOccurrences(of: "https://", with: "") + .replacingOccurrences(of: "http://", with: "") ?? "No server" + } // In SettingsView - .badge( - viewModel.esploraURL?.replacingOccurrences(of: "https://", with: "") - .replacingOccurrences( - of: "http://", - with: "" - ) ?? "No server" - ) + .badge(viewModel.formattedServerURL)
67-69: Improve timer management.Consider using a more robust approach for managing the copy feedback timer to prevent potential memory leaks.
+ @State private var feedbackTimer: Timer? - DispatchQueue.main.asyncAfter(deadline: .now() + 2) { - showCheckmark = false - } + feedbackTimer?.invalidate() + feedbackTimer = Timer.scheduledTimer(withTimeInterval: 2.0, repeats: false) { _ in + showCheckmark = false + }Don't forget to invalidate the timer when the view disappears:
.onDisappear { feedbackTimer?.invalidate() }
194-198: Enhance preview with different states.Consider adding more preview scenarios to help during development.
#if DEBUG - #Preview { - SettingsView(viewModel: .init()) + #Preview("Default") { + SettingsView(viewModel: .init()) + } + + #Preview("With Error") { + let viewModel = SettingsViewModel() + viewModel.nodeIDError = .init(title: "Error", detail: "Test error message") + return SettingsView(viewModel: viewModel) } #endif
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
LDKNodeMonday/View/Settings/SettingsView.swift(1 hunks)
🧰 Additional context used
📓 Learnings (1)
LDKNodeMonday/View/Settings/SettingsView.swift (1)
Learnt from: danielnordh
PR: reez/Monday#143
File: LDKNodeMonday/View/Settings/Danger/SeedView.swift:44-49
Timestamp: 2024-12-18T16:56:17.814Z
Learning: In SwiftUI for the LDKNodeMonday project, additional BIP39 validation for the seed mnemonic isn't strictly necessary because the node won't start unless the seed is valid.
🪛 SwiftLint (0.57.0)
LDKNodeMonday/View/Settings/SettingsView.swift
[Warning] 159-159: Prefer at least one space after slashes for comments
(comment_spacing)
🔇 Additional comments (1)
LDKNodeMonday/View/Settings/SettingsView.swift (1)
8-10: LGTM: Imports are appropriate and well-organized.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
LDKNodeMonday/View/Settings/Channel/ChannelDetailView.swift (1)
19-19: Remove redundant nil initializationThe explicit nil initialization is redundant for Optional variables in Swift.
- @State private var lastCopiedItemId: String? = nil + @State private var lastCopiedItemId: String?🧰 Tools
🪛 SwiftLint (0.57.0)
[Warning] 19-19: Initializing an optional variable with nil is redundant
(redundant_optional_initialization)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
LDKNodeMonday/View/Settings/Channel/ChannelDetailView.swift(2 hunks)LDKNodeMonday/View/Settings/Peers/PeerView.swift(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- LDKNodeMonday/View/Settings/Peers/PeerView.swift
🧰 Additional context used
🪛 SwiftLint (0.57.0)
LDKNodeMonday/View/Settings/Channel/ChannelDetailView.swift
[Warning] 19-19: Initializing an optional variable with nil is redundant
(redundant_optional_initialization)
🔇 Additional comments (3)
LDKNodeMonday/View/Settings/Channel/ChannelDetailView.swift (3)
15-15: LGTM: State management for channel closure
The addition of isClosing state variable improves the handling of channel closure state.
26-69: LGTM: Well-structured list implementation
The list implementation follows SwiftUI best practices with:
- Proper truncation for long values
- Visual feedback for copy operations
- Accessibility considerations with font sizing
74-84: LGTM: Improved navigation and accessibility
The changes improve the UI by:
- Limiting dynamic type size for better layout control
- Moving the close button to the navigation bar following iOS guidelines
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
LDKNodeMonday/View/Settings/SettingsView.swift (2)
98-155: 🛠️ Refactor suggestionEnhance safety measures for critical actions.
The current implementation uses basic confirmation alerts for critical actions. Consider implementing stronger safeguards.
struct SafetyConfirmationAlert: View { @Binding var isPresented: Bool let title: String let message: String let confirmationText: String @State private var userInput = "" let onConfirm: () -> Void var body: some View { VStack(spacing: 16) { Text(title).font(.headline) Text(message) Text("Type '\(confirmationText)' to confirm") .foregroundColor(.secondary) TextField("Confirmation", text: $userInput) .textFieldStyle(RoundedBorderTextFieldStyle()) HStack { Button("Cancel") { isPresented = false } Button("Confirm") { if userInput.uppercased() == confirmationText.uppercased() { onConfirm() isPresented = false } } .disabled(userInput.uppercased() != confirmationText.uppercased()) .foregroundColor(.red) } } .padding() } }
168-175:⚠️ Potential issueAdd error handling for initialization operations.
The
onAppearblock contains multiple async operations that should be wrapped in error handling..onAppear { Task { + do { viewModel.getNodeID() viewModel.getNetwork() viewModel.getEsploraUrl() await viewModel.getStatus() + } catch { + viewModel.nodeIDError = .init( + title: "Initialization Error", + detail: error.localizedDescription + ) + } } }
🧹 Nitpick comments (6)
LDKNodeMonday/View/Settings/Channel/ChannelDetailView.swift (3)
19-19: Remove redundant nil initializationThe explicit nil initialization of
lastCopiedItemIdis redundant as Optional variables are automatically initialized to nil.- @State private var lastCopiedItemId: String? = nil + @State private var lastCopiedItemId: String?🧰 Tools
🪛 SwiftLint (0.57.0)
[Warning] 19-19: Initializing an optional variable with nil is redundant
(redundant_optional_initialization)
40-44: Improve copy feedback implementationConsider using SwiftUI's native animation and a configuration constant for the feedback duration.
+ private let copyFeedbackDuration: TimeInterval = 2.0 Button { UIPasteboard.general.string = property.value - lastCopiedItemId = property.name - DispatchQueue.main.asyncAfter(deadline: .now() + 2) { + withAnimation { + lastCopiedItemId = property.name + } + withAnimation(.easeInOut(duration: copyFeedbackDuration)) { lastCopiedItemId = nil - } + } }
82-82: Consider using semantic colors for destructive actionsInstead of hardcoding the color red, consider using the system's semantic color for destructive actions.
- .foregroundColor(.red) + .foregroundColor(.red.opacity(0.85)) // or consider using .tint(.red) for better system integrationLDKNodeMonday/View/Settings/SettingsView.swift (3)
38-44: Consider extracting URL formatting logic.The nested string replacements could be simplified for better maintainability.
- .badge( - viewModel.esploraURL?.replacingOccurrences(of: "https://", with: "") - .replacingOccurrences( - of: "http://", - with: "" - ) ?? "No server" - ) + .badge(viewModel.formattedServerURL ?? "No server") // Add to SettingsViewModel: + var formattedServerURL: String? { + guard let url = esploraURL else { return nil } + return url.replacingOccurrences(of: "^(https?://)", with: "", options: .regularExpression) + }
63-68: Consider canceling previous copy timer.The current implementation might lead to unexpected behavior if the user taps the copy button multiple times quickly.
+ @State private var copyTimer: Timer? Button { UIPasteboard.general.string = viewModel.nodeID showCheckmark = true + copyTimer?.invalidate() - DispatchQueue.main.asyncAfter(deadline: .now() + 2) { - showCheckmark = false - } + copyTimer = Timer.scheduledTimer(withTimeInterval: 2.0, repeats: false) { _ in + showCheckmark = false + } }
158-158: Fix comment spacing.Add a space after the double slash for consistency with Swift style guidelines.
- //.scrollContentBackground(.hidden) // uncomment if we want white background + // .scrollContentBackground(.hidden) // uncomment if we want white background🧰 Tools
🪛 SwiftLint (0.57.0)
[Warning] 158-158: Prefer at least one space after slashes for comments
(comment_spacing)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
LDKNodeMonday/View/Settings/Channel/ChannelDetailView.swift(2 hunks)LDKNodeMonday/View/Settings/SettingsView.swift(1 hunks)
🧰 Additional context used
📓 Learnings (1)
LDKNodeMonday/View/Settings/SettingsView.swift (1)
Learnt from: danielnordh
PR: reez/Monday#143
File: LDKNodeMonday/View/Settings/Danger/SeedView.swift:44-49
Timestamp: 2024-12-18T16:56:17.814Z
Learning: In SwiftUI for the LDKNodeMonday project, additional BIP39 validation for the seed mnemonic isn't strictly necessary because the node won't start unless the seed is valid.
🪛 SwiftLint (0.57.0)
LDKNodeMonday/View/Settings/Channel/ChannelDetailView.swift
[Warning] 19-19: Initializing an optional variable with nil is redundant
(redundant_optional_initialization)
LDKNodeMonday/View/Settings/SettingsView.swift
[Warning] 158-158: Prefer at least one space after slashes for comments
(comment_spacing)
🔇 Additional comments (2)
LDKNodeMonday/View/Settings/Channel/ChannelDetailView.swift (1)
100-107: Previous review comment about channel closure implementation still applies
The current implementation was previously discussed and approved by the developer.
LDKNodeMonday/View/Settings/SettingsView.swift (1)
12-20: LGTM! Well-structured view properties.
The view properties follow SwiftUI best practices with proper use of property wrappers and clear naming conventions.
There was a problem hiding this comment.
This looks great!
Thanks for adding screenshots side by side and light+dark mode, always helps a bunch.
I like some of the details in here like the ChannelDetailsFormatted where you can use isCopyable in ChannelDetailView.
Thanks for formatting and removing some unneeded code now like getColor() etc.
Good stuff!
Updates the Settings view and associated subviews.
Follows previous style patterns established in the onboarding PR.
Summary by CodeRabbit
New Features
ChannelAddViewfor adding channels and aSeedViewfor displaying recovery phrases.Bug Fixes
Refactor
ChannelDetailViewandSettingsViewfor better user experience.NodeIDView,ChannelsListView, and others to streamline the interface.Documentation