refactor(ui): extract shared AmountEntryScreen and AmountEntryDelegate - #829
Merged
Conversation
github-actions
Bot
added
type: refactor
Extract duplicated amount-entry UI and ViewModel logic into a shared
module (apps/flipcash/shared/amount-entry). The new AmountEntryDelegate
encapsulates NumberInputHelper, currency observation, and animated model
management. The new AmountEntryScreen composable provides the unified
Column { AmountWithKeypad + CodeButton/SlideToConfirm } layout.
Migrated Cash (Give), Swap (Buy/Sell), and Withdrawal features to use
the shared components, eliminating ~250 lines of duplicated code.
Signed-off-by: Brandon McAnsh <git@bmcreations.dev>
…Delegate AmountEntryDelegate now derives AmountEntryConfig internally from style, loadingState, maxAmount, and minimumAmount flows passed at construction. VMs provide structured AmountEntryStyle inputs instead of owning a secondary config StateFlow, eliminating the only instance of a secondary StateFlow on any BaseViewModel in the codebase. Signed-off-by: Brandon McAnsh <git@bmcreations.dev>
bmc08gt
force-pushed
the
refactor/shared-amount-entry
branch
2 times, most recently
from
June 3, 2026 14:56
3dc825f to
fa75fbd
Compare
…estability AmountEntryDelegate now implements AmountEntryController, which captures the screen-facing API (state, config, keypad input). AmountEntryScreen takes the interface instead of the concrete class, enabling Compose UI tests with a trivial FakeController — no Exchange mock, coroutine scope, or flow wiring required. Adds 10 Robolectric Compose UI tests covering action labels, enabled state, loading transitions, confirm callback, slide variant, hints, and keypad forwarding. Signed-off-by: Brandon McAnsh <git@bmcreations.dev>
bmc08gt
force-pushed
the
refactor/shared-amount-entry
branch
from
June 3, 2026 14:58
fa75fbd to
e319a6b
Compare
…, and WithdrawalConfirmation Add Robolectric-based Compose UI tests following the established pattern from AmountEntryScreenTest. Includes receipt, currency conversion, and bonding curve token rendering tests. Extract compose-ui-testing bundle and enable isIncludeAndroidResources globally in the convention plugin. Signed-off-by: Brandon McAnsh <git@bmcreations.dev>
…Model API Signed-off-by: Brandon McAnsh <git@bmcreations.dev>
bmc08gt
marked this pull request as ready for review
June 3, 2026 15:27
Gradle 9 fails test tasks when test dependencies exist but no test classes are discovered. Set failOnNoDiscoveredTests to false in the convention plugin since kotlin-test-junit is added to all modules.
Keep refactored AmountEntryScreen usage, discard old AmountWithKeypad inline implementation that was replaced by the shared module.
github-actions
Bot
added
area: onramp
Include :libs:test-utils as a testImplementation dependency in the base library convention plugin so all modules pick up the robolectric.properties file pinning SDK to 36. Remove the now-redundant manual declarations from 15 individual modules.
bmc08gt
force-pushed
the
refactor/shared-amount-entry
branch
from
June 5, 2026 13:06
a058b94 to
e9bc21d
Compare
bmc08gt
added a commit
that referenced
this pull request
Aug 20, 2026
The private `netTransferAmount` producer deferred to `State.confirmedNetTransferAmount` — its own output from a previous trip through the entry screen — while `enteredAmount` and `feeAmount` beside it recomputed from the live amount delegate. All three feed the same `OnAmountAccepted` event, so a second pass produced a receipt that mixed the new entry with the old total: enter $1, go back, enter $0.50, and the receipt paired a $0.50 debit and a $0.005 fee with a $0.99 "You Receive". The cache-first read is a leftover from before #829, when a single getter on `State` served both roles and the keypad lived on `State` too; there it was what pinned the number once accepted. The delegate split gave that freeze its own home in `State.netTransferAmount`, but the producer copy kept the prefix. `enteredAmount` and `feeAmount` were rewritten during that split and came out cache-free, which is the asymmetry behind the bug. Affects v1's Sell path the same way, so it is left ungated — a wrong number is not a v1 pixel worth preserving.
bmc08gt
added a commit
that referenced
this pull request
Aug 20, 2026
* feat(convert): currency-to-currency Convert flow Convert moves value between two currencies the user already holds: a destination picker on the amount-entry screen, a receipt that names both sides, and the swap plumbing behind it. The processing title reads a plain "Converting" rather than naming the destination — the destination is already on the receipt the user just confirmed, and long currency names truncated in the app bar. The receipt shows token logos rather than currency flags, matching the withdrawal and Get receipts; a flag can't tell two currencies apart when they share one (or have none). * feat(convert): v2 amount header + Select Currency sheet Bring the Convert flow in line with the iOS spec: - New `displayExtraLarge` (74sp bold) type token, and map `textTertiary` to the real tertiary grey rather than White50. - `AmountArea`/`AmountTextAnimated` gain a content colour and a horizontal alignment, so the amount can sit start-aligned with the prefix and the un-entered placeholder digits rendered in tertiary grey. - `AmountWithKeypad` grows a `largeHeader` layout: the amount is top-anchored and left-aligned at display-extra-large over an "$X available" line, which simply turns red once the entry exceeds the balance instead of swapping in a separate over-limit sentence. - The Convert destination picker is now a bottom sheet rather than a pushed step: `SwapStep.ConvertDestinationSelection` is marked as a `Sheet` and the swap `FlowHost` gets the modal-sheet scene strategy. Because that is an overlay scene, amount entry stays composed underneath and never re-runs its entry effects. - Restrict Convert destinations to currencies the user already holds, matching iOS — acquiring something new is a Get, not a Convert. * feat(nav): half-detent sheets for the currency picker Adds a half-detent option to the modal sheet scene and points the select-currency step at it, so the picker opens at half height and sizes its list to the detent instead of the full screen. * feat(sheets): currency sheet to spec — self-sizing height and edge fade Brings the select-currency sheet to spec: row metrics, the list's edge fade, and a height the sheet itself owns. Height is the sheet's business, not the content's — restating a fraction of the screen in the content capped the list short of the sheet's own bottom edge and stranded the fade above it. The sheet now expands only when its content needs more room than the detent gives. * refactor(ui): slot-based AmountWithKeypad Turns AmountWithKeypad into a slot-based component so callers supply the accessory row rather than the component guessing at it, and gives the v2 layout the gap above the amount field that the spec calls for — including on small screens, where a bare Spacer under heightIn(max=) measured to the minimum and collapsed. * feat(tokens): v2 "Get" refresh — inline funding source + conversion-style receipt Reframes Get as a conversion between two currencies the user holds: the funding source is picked inline on the amount-entry screen instead of on a separate step, and the receipt reads "You Get / Amount to convert / Conversion fee / You Pay". The accessory token chip cross-fades when the selection changes rather than popping. * refactor(tokens): one receipt screen behind Get and Convert Get and Convert had separate receipt screens that had converged on the same layout: the same bordered card, the same anchor rows, the same scaffold and bottom bar. Everything still differing between them was data — which side leads, the warning copy, the confirm label and its progress state — so it now arrives as data, and the fee math the two genuinely disagree on stays in its own adapter. Two knock-on changes: Convert picks up Get's shimmer, so pending token or amount data no longer blanks the screen instead of rendering it, and the line items keep Convert's spacing (Get's was 5dp tighter). The token name label under each amount is gone from both. Fee lines test hasDisplayableValue rather than a hardcoded 0.01, which was USD-shaped: a sub-unit fee in another currency now reads "~ ¥0" instead of a bare "¥0". * fix(nav): predictive back runs the pop animation, not the push AppNavHost defaulted predictivePopTransitionSpec to transitionSpec — the *forward* spec — so dragging back out of a flow step slid the screen being returned to in from the right instead of the left. Every other host passes the parameter explicitly and so hid the bad default; FlowHost did not, which is why backing out of the Get/Convert receipts ran the wrong way. Default it to popTransitionSpec and thread it through FlowHost. Hosts that don't distinguish push from pop are unaffected: for them the two specs are the same lambda. * fix(tokens): recompute the net transfer on re-entry The private `netTransferAmount` producer deferred to `State.confirmedNetTransferAmount` — its own output from a previous trip through the entry screen — while `enteredAmount` and `feeAmount` beside it recomputed from the live amount delegate. All three feed the same `OnAmountAccepted` event, so a second pass produced a receipt that mixed the new entry with the old total: enter $1, go back, enter $0.50, and the receipt paired a $0.50 debit and a $0.005 fee with a $0.99 "You Receive". The cache-first read is a leftover from before #829, when a single getter on `State` served both roles and the keypad lived on `State` too; there it was what pinned the number once accepted. The delegate split gave that freeze its own home in `State.netTransferAmount`, but the producer copy kept the prefix. `enteredAmount` and `feeAmount` were rewritten during that split and came out cache-free, which is the asymmetry behind the bug. Affects v1's Sell path the same way, so it is left ungated — a wrong number is not a v1 pixel worth preserving.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Extract duplicated amount-entry UI and ViewModel logic into a shared module (apps/flipcash/shared/amount-entry). The new AmountEntryDelegate encapsulates NumberInputHelper, currency observation, and animated model management. The new AmountEntryScreen composable provides the unified Column { AmountWithKeypad + CodeButton/SlideToConfirm } layout.
Migrated Send, Cash (Give), Swap (Buy/Sell), and Withdrawal features to use the shared components, eliminating ~250 lines of duplicated code.