-
Notifications
You must be signed in to change notification settings - Fork 1
fix: various fixes #289
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: various fixes #289
Conversation
14de260 to
e1af5f3
Compare
e1af5f3 to
3a00f37
Compare
3a00f37 to
0be7feb
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR addresses three distinct issues: toast notifications not appearing after app crashes, incorrect maximum sendable amounts for onchain invoices, and deletion behavior problems in the number pad when using spaces as digit separators.
Key Changes:
- Added crash recovery logic to ensure toast window is properly recreated after soft crashes
- Fixed max sendable amount calculation to recalculate when fee rates change and handle errors properly
- Corrected number pad deletion behavior by removing spaces from raw input for modern Bitcoin display
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| BitkitTests/NumberPadTests.swift | Added test coverage for delete functionality with formatted text containing spaces |
| Bitkit/Views/Wallets/Send/SendAmountView.swift | Added fee rate change listener and fixed error handling for max amount calculation |
| Bitkit/Views/Settings/DevSettingsView.swift | Added crash simulation button for testing crash recovery |
| Bitkit/ViewModels/AmountInputViewModel.swift | Fixed raw input text handling to remove spaces for modern Bitcoin display |
| Bitkit/Managers/ToastWindowManager.swift | Enhanced window lifecycle management and crash recovery for toast notifications |
| .onChange(of: wallet.selectedFeeRateSatsPerVByte) { _ in | ||
| // Recalculate max sendable amount when fee rate becomes available or changes | ||
| if app.selectedWalletToPayFrom == .onchain { | ||
| Task { | ||
| await calculateMaxSendableAmount() | ||
| } | ||
| } | ||
| } |
Copilot
AI
Dec 24, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The onChange closure uses an unnamed parameter '_' but never checks if the fee rate is valid or non-zero. Consider adding a guard statement to verify the fee rate is set before recalculating: guard wallet.selectedFeeRateSatsPerVByte > 0 else { return }
| existingWindow.windowScene != nil, | ||
| !existingWindow.isHidden |
Copilot
AI
Dec 24, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The condition checks if the window is not hidden, but a valid window could be hidden and still functional. Consider removing the !existingWindow.isHidden check, as the window should be shown when needed regardless of its current hidden state.
| existingWindow.windowScene != nil, | |
| !existingWindow.isHidden | |
| existingWindow.windowScene != nil |
| for scene in UIApplication.shared.connectedScenes { | ||
| if let windowScene = scene as? UIWindowScene, | ||
| windowScene.activationState == .foregroundActive || windowScene.activationState == .foregroundInactive |
Copilot
AI
Dec 24, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The activation state check could be simplified and made more maintainable by storing the valid states in an array. Consider: let validStates: [UIScene.ActivationState] = [.foregroundActive, .foregroundInactive] and then validStates.contains(windowScene.activationState)
| for scene in UIApplication.shared.connectedScenes { | |
| if let windowScene = scene as? UIWindowScene, | |
| windowScene.activationState == .foregroundActive || windowScene.activationState == .foregroundInactive | |
| let validStates: [UIScene.ActivationState] = [.foregroundActive, .foregroundInactive] | |
| for scene in UIApplication.shared.connectedScenes { | |
| if let windowScene = scene as? UIWindowScene, | |
| validStates.contains(windowScene.activationState) |
jvsena42
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested:
- Max send amount
- Switch between classic and modern input
Description