feat(predict): Implement Predict Bet Slip PRED-707 cp-7.74.0#28779
Conversation
|
CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
github-actions
Bot
added
the
risk-high
github-actions
Bot
added
risk-medium
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #28779 +/- ##
==========================================
+ Coverage 82.19% 82.24% +0.05%
==========================================
Files 5043 5048 +5
Lines 132426 132759 +333
Branches 29571 29698 +127
==========================================
+ Hits 108848 109191 +343
+ Misses 16172 16159 -13
- Partials 7406 7409 +3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
github-actions
Bot
added
risk-medium
github-actions
Bot
added
risk-medium
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
github-actions
Bot
added
risk-medium
github-actions
Bot
added
risk-medium
| transactionId: | ||
| currentState === ActiveOrderState.PAY_WITH_ANY_TOKEN | ||
| ? approvalRequest?.id | ||
| : undefined, |
There was a problem hiding this comment.
Non-awaited onApprovalConfirm falls through to handlePlaceOrder immediately
Medium Severity
When currentState is PAY_WITH_ANY_TOKEN and approvalRequest?.id exists, onApprovalConfirm is called without await at line 180-184, then execution falls through to handlePlaceOrder at line 209. The new else branch (re-init path) correctly returns early, but the existing if branch doesn't — it fires the approval asynchronously and immediately proceeds to place the order. In the new sheet mode, this race is more visible because the sheet unmount cleanup can run before the approval completes.
Reviewed by Cursor Bugbot for commit 5c98134. Configure here.
|
Added skip sonar cloud label due to it flagging other files consistently for 1 line changes that are out of scope |
| const route = | ||
| useRoute<RouteProp<PredictNavigationParamList, 'PredictBuyPreview'>>(); | ||
|
|
||
| const { market, outcome, outcomeToken, entryPoint } = route.params; |
There was a problem hiding this comment.
Sheet mode Buy component calls useRoute outside Screen context
Medium Severity
Both PredictBuyWithAnyToken and PredictBuyPreview call useRoute() unconditionally at the top of the component. When rendered in sheet mode, these components are children of PredictPreviewSheetProvider, which sits outside the Stack.Navigator's Screen components. useRoute() requires being inside a screen's route context. If the PredictScreenStack component itself isn't rendered inside a parent navigator screen (or if the nearest route context lacks params), useRoute() could throw or return unexpected params. While the result isn't accessed in sheet mode, the hook call itself is the risk point.
Additional Locations (2)
Reviewed by Cursor Bugbot for commit 642ee64. Configure here.
| /> | ||
| )} | ||
| <PredictBuyBottomContent | ||
| isInputFocused={isSheetMode ? false : isInputFocused} |
There was a problem hiding this comment.
Sheet-mode bottom content always visible hides keypad interaction
Medium Severity
In sheet mode, PredictBuyBottomContent receives isInputFocused={false} (hardcoded), making it always visible, while the PredictKeypad (rendered below the bottom content) uses the real isInputFocused value. Since isInputFocused initializes to true, both the full bottom content (quick amounts, pay row, fee summary, confirm button) and the keypad render simultaneously on first open. The keypad's hideHeader prop removes the "Done" button, so the only way to dismiss the keypad is to tap a PredictQuickAmounts button — which overwrites any custom-typed amount. There is no way for the user to confirm a manually-typed value and dismiss the keypad without losing their input.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 78efa93. Configure here.
🔍 Smart E2E Test Selection
click to see 🤖 AI reasoning detailsE2E Test Selection: Key changes:
Tag selection rationale:
Performance Test Selection: |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
There are 6 total unresolved issues (including 4 from previous reviews).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit ca83caf. Configure here.
| expect(getByText('Share address')).toBeOnTheScreen(); | ||
| }); | ||
|
|
||
| jest.useFakeTimers(); |
There was a problem hiding this comment.
Timer mode leak on test failure breaks isolation
Low Severity
jest.useRealTimers() is called at test start and jest.useFakeTimers() at test end, but if waitFor times out and throws, the cleanup line is skipped. Subsequent tests would then run with real timers instead of fake timers, causing hard-to-diagnose cascading failures. The cleanup belongs in a try/finally block or an afterEach to guarantee timer mode is restored regardless of test outcome.
Reviewed by Cursor Bugbot for commit ca83caf. Configure here.
| ); | ||
|
|
||
| expect(screen.getByText('Confirm')).toBeOnTheScreen(); | ||
| expect(screen.queryByText(/· /)).toBeNull(); |
There was a problem hiding this comment.
New test assertions use toBeNull not toBeOnTheScreen
Low Severity
Newly added test assertions use .toBeNull() to check element absence (e.g., expect(screen.queryByText(...)).toBeNull()). The unit testing guidelines explicitly require not.toBeOnTheScreen() instead of toBeNull() for element presence/absence checks. The PR description itself states "Replaced all toBeNull() assertions with not.toBeOnTheScreen()" yet these new lines introduce fresh violations.
Additional Locations (1)
Triggered by project rule: Unit Testing Guidelines
Reviewed by Cursor Bugbot for commit ca83caf. Configure here.
|
✅ E2E Fixture Validation — Schema is up to date |
|
4 participants
Description
Migrate the Predict Buy and Sell preview screens from full-screen stack navigation into BottomSheet wrappers, gated behind the
predictBottomSheetLaunchDarkly feature flag. When the flag is OFF, the existing full-screen navigation flow is preserved with zero behavioral changes.What changed
New:
PredictPreviewSheetwrapper componentBottomSheet,BottomSheetHeader, andBottomSheetHeaderVariantfrom@metamask/design-system-react-native(not the deprecated component-library version).renderHeaderprop for custom header content (sell sheet uses it for rich cashout info); falls back to default icon+title+subtitle layout (buy sheet).isFullscreenprop controls sheet height: content-fitted for both buy and sell sheets (auto-sizes to content).testIDprops (preview-sheet-title,preview-sheet-subtitle) for testID-based assertions.New:
PredictPreviewSheetContext(central sheet state manager)openBuySheet()/openSellSheet()methods consumed by all card components.SellSheetHeaderextracted as a standalone component within the context for the rich header.usePredictPreviewSheet()returns navigation-based routing when used outside the provider (e.g. home carousel, trending feed) instead of throwing, with memoized references to prevent unnecessary re-renders.New:
predictBottomSheetfeature flagfeature-flag-registry.tsas a remote, version-gated flag (default: disabled).selectPredictBottomSheetEnabledFlaginselectors/featureFlags/index.ts.New:
PredictQuickAmountscomponent (with haptic feedback)expo-haptics(impactAsyncwithImpactFeedbackStyle.Light) for tactile feedback on button press.impactAsyncis properly awaited with error swallowing for unsupported devices."20"not"20.00") for easier editing.Boxcomponent instead of rawView.New:
usePredictCashOuthook (shared cashout logic)position.outcomeId, callopenSellSheet, and handle errors withLogger.error+ toast notification.marketandcallerNameparameters;callerNameis included in error metadata for production diagnostics.PredictPicksandPredictPositionDetail, eliminating ~40 lines of identical code from each.hooks/index.ts.openSellSheetparams, error/toast on missing outcome, andcallerNamein metadata.Navigation migration (10 components updated)
All components that previously called
navigateToBuyPreview()/navigate(Routes.PREDICT.MODALS.SELL_PREVIEW)now callopenBuySheet()/openSellSheet()fromusePredictPreviewSheet:PredictMarketDetails- outcome buy actionsPredictMarketSingle- Yes/No buy buttonsPredictMarketMultiple- outcome buy buttonsPredictMarketOutcome- price buttonsPredictSportCardFooter- feed card buy actionsFeaturedCarouselCard- carousel card outcome buy buttonsFeaturedCarouselSportCard- sport carousel card buy buttonsPredictPicks- cashout from picks list via sharedusePredictCashOuthookPredictPositionDetail- cashout from position detail via sharedusePredictCashOuthookusePredictNavigationfrom all these components.Routes (
routes/index.tsx)PredictPreviewSheetProviderso sheets are available throughout the Predict navigation tree.Buy sheet (
PredictBuyWithAnyToken)mode: 'sheet' | 'screen') for type-safe sheet vs navigation mode, replacingPartial<ContentProps>withascasts.PredictPreviewSheet),PredictQuickAmountsin bottom content,PredictPayWithRowin compactvariant="row"mode with balance display, keypad moved below bottom content withhideHeader, border hidden, action button displays "Confirm".Boxwrapper instead ofSafeAreaViewin sheet mode.PredictPayWithAnyTokenInforeceivesisInputFocused: falsein sheet mode to ensure mm_pay relay configuration runs immediately (prevents underfunded deposit).onReject+clearActiveOrderTransactionId(matchesbeforeRemovebehavior in old flow).approvalRequestis missing duringhandleConfirmin PAY_WITH_ANY_TOKEN state, attempts re-initialization viainitPayWithAnyToken()(clearsbatchIdRef, rejects pending transactions, re-creates the approval) before returningPLACE_ORDER_FAILED. This gives the user a chance to retry with a fresh approval rather than silently failing.Buy sheet (
PredictBuyPreview- legacy non-pay-with-any-token)isSheetModepattern for BottomSheet compatibility.onClose().ScrollViewfromreact-native-gesture-handler(aliased asGHScrollView) in sheet mode to cooperate with the BottomSheet'sPanGestureHandler; standard React NativeScrollViewpreserved for the full-screen path.Sell sheet (
PredictSellPreview)isSheetModedrives conditional layout: icon+title row hidden in sheet mode (rendered bySellSheetHeaderin the sheet header instead), price/shares/PnL section relocated to the bottom area.tw.style('flex-col')instead ofStyleSheet.create()for the container.getCashoutInfoTexthelper intoutils/format.tsto DRY up the localized cashout info string.PredictController(batch transaction fix)gasFeeToken: MATIC_CONTRACTS.collateralto theinitPayWithAnyTokenbatch submission, matching other batch calls. This ensures mm_pay configures the relay step to bridge funds when the Safe has insufficient USDC balance.PredictKeypadhideHeaderprop to suppress the quick-amount row + Done button when rendered inside the buy sheet (avoids duplication withPredictQuickAmounts).PredictPayWithRowvariant="row"mode: compact single-row layout showing token icon, symbol, balance, and right chevron for the sheet.availableBalanceprop for displaying balance in row mode.PredictBuyAmountSectionhideAvailableBalanceprop to suppress the "Available: $X.XX" text in sheet mode.PredictBuyBottomContenthideBorderprop to remove the top border separator in sheet mode.Textinstead of sibling flex items).PredictBuyActionButtonisSheetModeprop: when true, displays "Confirm" label instead of "{outcome} · {price}".PredictFeeSummaryHeadingMd/BoldtoBodyMd/Mediumto match Pay With row.Bug fixes
usePredictBuyConditions: Auto-revert effect that switched back to Predict balance now requirestotalPayForPredictBalance > 0, preventing immediate revert when the amount is 0 (which always caused the payment method selection to snap back in sheet mode).usePredictBuyConditions:shouldWaitForPayFeesnow also requirescurrentValue > 0.usePredictBuyActions: WhenapprovalRequestis missing duringhandleConfirmin PAY_WITH_ANY_TOKEN state, attempts re-initialization (initPayWithAnyToken()) before returning early with error, giving the user a retry path rather than silently failing. Removed dead fallback branch that was unreachable (transactionId derived from the same approvalRequest).usePredictBuyActions:batchIdRefcleared at the start ofdoInit()to prevent stale values between transaction attempts.usePredictBuyActions: Sheet unmount cleanup mirrorsbeforeRemovebehavior from the old flow.usePredictBuyActions: DEPOSITING and SUCCESS state effects checkisSheetModeto callonClose()instead ofStackActions.pop().usePredictBuyError: RemovedisInputFocusedfrom theblockingPayAlertMessagegate. The flag was permanently suppressing insufficient-funds errors in sheet mode becauseisInputFocusedinitialized totrueand never transitioned tofalsewhen using quick-amount buttons. TheisPayFeesLoadingcheck already handles stale-data concerns.PredictPicks/PredictPositionDetail: Cashout logic (outcome lookup,openSellSheet, error handling withLogger.error+ toast) extracted into sharedusePredictCashOuthook to eliminate duplication.PredictPayWithAnyTokenInfo:isInputFocusedoverridden tofalsein sheet mode soupdatePendingAmountandsetPayTokenrun immediately, ensuring mm_pay relay step is configured before confirmation.PredictController: AddedgasFeeTokentoinitPayWithAnyTokenbatch to fix missing mm_pay relay step that caused "Insufficient USDC balance in Safe" errors.PredictPreviewSheetContext: StabilizedonDismisscallbacks withuseCallbackto prevent unstablecloseSheet/onClosereferences from causing repeated SUCCESS/DEPOSITING effect re-fires.Code quality
usePredictCashOuthook to deduplicate identicalonCashOutlogic (~40 lines each) fromPredictPicksandPredictPositionDetail. Both components now use a single hook call instead of duplicating the guarded action, outcome lookup,openSellSheetcall, and error handling withLogger.error+ toast. ThecallerNameparameter preserves per-component error metadata.Viewwith design-systemBoxinPredictQuickAmounts.usePredictBottomSheet.PredictBuyPreviewProps,PredictSellPreviewProps) replacePartial<ContentProps>withascasts, providing compile-time safety for sheet vs navigation mode.BUY_PREVIEW_SHEET,SELL_PREVIEW_SHEET.usePredictPreviewSheetto prevent unstable function references.BuyComponentselection inPredictPreviewSheetContextwithuseMemoto prevent unnecessary unmount/remount cycles.index.tsbarrel export forPredictPreviewSheetcomponent (file organization standard).cardStyleInterpolatoronBUY_PREVIEWandSELL_PREVIEWstack screens for the flag-OFF full-screen path.require()withjest.requireActual()in all test mock factories.navigationRef) inroutes/index.test.tsx— moved tobeforeEach.PredictMarketDetails.test.tsxto verify exact call parameters withexpect.objectContaining().predict.order.buyi18n key and its stale test mock reference.PredictPreviewSheetfrom deprecated component-libraryBottomSheetto@metamask/design-system-react-nativeBottomSheet/BottomSheetHeader(DS-first UI rule). UpdatedusePredictBottomSheethook to importBottomSheetReffrom DS.shouldNavigateBack={false}with DS equivalent (omitgoBackprop); changed headerstyleprop totwClassName.PredictBuyPreview: Uses gesture-handler-awareScrollViewfromreact-native-gesture-handlerin sheet mode to prevent scroll/dismiss conflicts with the BottomSheet'sPanGestureHandleron Android.toBeNull()assertions withnot.toBeOnTheScreen()in test files per unit-test guidelines.getByText,queryByText) with testID-based assertions (getByTestId+toHaveTextContent) inPredictPreviewSheet.test.tsxandPredictPreviewSheetContext.test.tsx.act()toawait act(async () => { ... })inroutes/index.test.tsxto flush async navigator state updates and eliminate act() warnings.Localization
predict.oddskey ("Odds").predict.order.confirmkey ("Confirm") for the sheet action button.predict.order.buykey ("Buy {{outcome}}").Documentation
docs/predict/predictions-comprehensive-guide.md).docs/predict/tickets/buy-sell-bottomsheet-migration.md).Changelog
CHANGELOG entry: null
Related issues
Refs: PRED-707
Manual testing steps
Screenshots/Recordings
Before
Full-screen buy/sell preview navigation (flag OFF) - no visual changes to this flow.
After
newDemoPRedict.mov
Pre-merge author checklist
Pre-merge reviewer checklist
Note
Medium Risk
Touches Predict trade entry points and preview/cash-out flows, plus introduces a new feature-flagged UI layer; regressions could affect order preview UX or sheet/navigation behavior, though the legacy route path remains as fallback.
Overview
Adds a new feature-flagged bet slip flow for Predict by introducing
PredictPreviewSheet+PredictPreviewSheetProvider/usePredictPreviewSheet, allowing buy/sell previews to open in BottomSheets (with navigation fallback when the provider/flag isn’t available).Updates multiple Predict entry points (feed cards, carousel cards, market single/multiple/outcome components, and sport footer) to call
openBuySheetinstead of navigating to the buy preview, and refactors cash-out handling into a sharedusePredictCashOuthook that opens the sell sheet and handles missing-outcome errors via logging + toast.Extends buy preview screens (
PredictBuyPreview,PredictBuyWithAnyToken) to support a newmode: 'sheet'rendering path (layout tweaks, keypad/header behavior, confirm CTA), adds new test IDs/feature-flag selectors (predictBottomSheet), and adjusts provider order result shape and assorted tests to match the new behavior.Reviewed by Cursor Bugbot for commit ca83caf. Bugbot is set up for automated code reviews on this repo. Configure here.