refactor: AmountInputViewModel to @Observable#586
Open
CypherPoet wants to merge 5 commits into
Open
Conversation
4 tasks
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 012ae40175
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
012ae40 to
1d13030
Compare
The send number pad now rejects keystrokes that would push the amount above the available sendable balance, reusing the existing over-max block (haptic + error flash) via a dynamic maxAmountOverride. Continue-button validation is unchanged as a backstop. Closes synonymdev#346
1d13030 to
64655dc
Compare
The cap rejected every keystroke whose result still exceeded it, including deletions. When an amount lands above the cap (a prefilled invoice over the available balance, or a cap that dropped after input), the user could not backspace to reduce it, since each intermediate value was still over the cap. Deletions now always apply; the cap only blocks growing the amount.
Applies the maxAmountOverride cap (from the send-amount fix) to the LNURL pay/withdraw, spending, advanced-spending, and manual channel-funding screens, so the number pad refuses entry above each screen's contextual maximum. FundManualAmountView also gains the previously-missing upper-bound button validation, which let users proceed above the fundable balance. Receive and CJIT amount screens are intentionally left uncapped (a receive amount has no balance ceiling).
Replaces the legacy ObservableObject/@StateObject/@ObservedObject pattern with @observable + @State, aligning with the project's stated SwiftUI direction. No behavior change: there are no two-way bindings to the view model, so no @bindable is needed. Touches the view model, NumberPadTextField, and the 8 amount-entry call sites.
64655dc to
9bcbe3a
Compare
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
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.
Depends on
Depends on #584 and #585; a no-behavior
@Observablerefactor stacked on both. Merge #584 then #585 first. This PR targetsmaster, so until they land its diff also contains their commits; review therefactor: migrate AmountInputViewModel to @Observablecommit (012ae401) for this PR's changes.Description
This PR migrates AmountInputViewModel from the legacy ObservableObject pattern to the @observable macro, matching the project's stated SwiftUI direction. There is no behavior change.
The view model swaps ObservableObject and @published for @observable, NumberPadTextField drops @ObservedObject, and the eight amount-entry screens move from @StateObject to @State. No @bindable is needed anywhere, since nothing creates two-way bindings to the view model; every consumer reads its values or calls its methods.
Linked Issues/Tasks
Follows up #346 and the amount-screen cap work.
Screenshot / Video
N/A; no behavior change.
QA Notes
Manual Tests
regression:Send -> Amount: typing updates the displayed amount and fiat conversion; the cap still works.regression:Receive -> Amount: typing updates the invoice amount.regression:Transfer -> Spending Amount: typing updates and caps as before.regression:LNURL Pay / Withdraw -> Amount: typing and cap behave as before.Automated Checks
BitkitTests/NumberPadTestspasses (24 tests).