fix(perps): eliminate trade screen slider lag and flicker - #33590
Conversation
Rewrite PerpsSlider as a thin wrapper around the MetaMask Design System Slider instead of the custom react-native-gesture-handler + reanimated implementation, which flooded the JS thread with runOnJS calls and expensive fee/rewards recomputes on every drag frame (TAT-3543). PerpsOrderView and PerpsClosePositionView now split slider state into a cheap live display value and a value committed only on drag end. Co-authored-by: Cursor <cursoragent@cursor.com>
|
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. |
PR template — items to address before "Ready for review"Warnings — informational, address before merging:
See docs/readme/ready-for-review.md for the full Definition of Ready for Review. |
…ider-lag Co-authored-by: Cursor <cursoragent@cursor.com> # Conflicts: # app/components/UI/Perps/Views/PerpsClosePositionView/PerpsClosePositionView.tsx # app/components/UI/Perps/components/PerpsSlider/PerpsSlider.styles.ts # app/components/UI/Perps/components/PerpsSlider/PerpsSlider.test.tsx # app/components/UI/Perps/components/PerpsSlider/PerpsSlider.tsx
Picks up MetaMask/metamask-design-system#1397, a fix for a stale controlled-value echo bug in useSliderGesture that caused the DS Slider's thumb to visibly rewind/flicker on rapid taps or fast pans. Complementary to this branch's PerpsSlider rewrite, which fixes the JS-thread-flooding root cause of the perps trade-screen slider lag. Co-authored-by: Cursor <cursoragent@cursor.com>
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
…omponent-flow integration harness The DS Slider's useSliderGesture calls Gesture.Pan().onStart(...), which this harness's react-native-gesture-handler mock didn't provide (it only covered the old custom PerpsSlider's onBegin/onUpdate/onEnd/onFinalize chain), crashing componentFlow.integration.test.tsx when PerpsOrderView renders the real PerpsSlider -> DS Slider. Co-authored-by: Cursor <cursoragent@cursor.com>
abretonc7s
left a comment
There was a problem hiding this comment.
Please address the inline comments.
| }} | ||
| value={parseFloat(displayAmount || '0')} | ||
| onValueChange={handleSliderValueChange} | ||
| onDragEnd={handleSliderDragEnd} |
There was a problem hiding this comment.
A cancelled gesture skips onDragEnd, leaving the displayed amount ahead of the committed/submitted amount. Please commit or reset on cancellation.
| setDisplayClosePercentage(value); | ||
| }; | ||
|
|
||
| const handleSliderDragEnd = (value: number) => { |
There was a problem hiding this comment.
Please memoize both slider callbacks; each drag tick currently recreates the slider gestures mid-drag.
| // slippage recompute pipeline (usePerpsOrderFees et al.). | ||
| const [displayAmount, setDisplayAmount] = useState(orderForm.amount); | ||
|
|
||
| useEffect(() => { |
There was a problem hiding this comment.
This syncs after paint, so non-slider edits briefly render stale amounts. Please derive from the committed amount when not dragging or update both states synchronously.
- Derive displayAmount/displayClosePercentage from a dragging flag instead of syncing via useEffect, so keypad/percentage/max/clamp/ leverage edits render the committed value immediately instead of lagging a render behind (Bugbot + review comment). - Memoize PerpsClosePositionView's slider callbacks with useCallback so they no longer rebuild mid-drag (review comment). - Reset the dragging flag on onTouchCancel in both views so a gesture cancelled before onDragEnd fires falls back to the last committed value instead of leaving the display stuck ahead of it (review comment). Co-authored-by: Cursor <cursoragent@cursor.com>
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #33590 +/- ##
==========================================
+ Coverage 84.80% 84.82% +0.01%
==========================================
Files 6254 6256 +2
Lines 168765 168778 +13
Branches 41306 41300 -6
==========================================
+ Hits 143126 143167 +41
+ Misses 15870 15839 -31
- Partials 9769 9772 +3 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
…e touch-cancel react-native-gesture-handler owns the touch outside RN's responder system, so a gesture cancelled by competing-gesture arbitration (e.g. a parent ScrollView taking over mid-drag) does not reliably bubble an RN touch-cancel event, leaving isDraggingSlider stuck true and the display ahead of the committed amount. Replace the onTouchCancel-only reset with a restart-on-tick stall timer that commits the last live value once ticks stop arriving, keeping onTouchCancel as a best-effort immediate signal. Co-authored-by: Cursor <cursoragent@cursor.com>
… reset Bugbot correctly flagged the stall-timeout safety net as unsound: ticks only arrive when the stepped value changes, so a normal mid-drag pause (or a hold on the same step) is indistinguishable from a cancelled gesture, causing commitDragAmount/commitDragClosePercentage to fire while the finger is still down and kicking off the fee/rewards/slippage pipeline early. Replace it with a single commit funnel (commitAmount / commitClosePercentage) that every input path — slider drag end, keypad, percentage, max, and leverage clamp — routes through, unconditionally clearing isDraggingSlider. This removes the false-positive risk entirely: a stuck flag from a gesture that skips onDragEnd now self-heals the instant the user does anything else, with no heuristic guessing about gesture state. onTouchCancel remains as a best-effort immediate signal. Also guard place-order/confirm-close submission itself: if isDraggingSlider is somehow still true at submit time, flush the live value and bail rather than risk submitting a stale committed amount. Co-authored-by: Cursor <cursoragent@cursor.com>
… clamps commitAmount only ran when the new leverage required clamping the amount down, so a leverage change that didn't need clamping left isDraggingSlider (and the comments describing it) inaccurate: the live drag value kept shadowing the correct, unchanged orderForm.amount (Bugbot). Co-authored-by: Cursor <cursoragent@cursor.com>
…iscard it setIsDraggingSlider(false) alone discarded a pending liveDragAmount back to the stale pre-drag orderForm.amount, and the clamp check read that same stale value — unlike handleSliderDragCancel and the place-order guard, which both flush the live value forward instead of dropping it (Bugbot). Co-authored-by: Cursor <cursoragent@cursor.com>
…lamp paths SonarCloud's new-code coverage gate failed (55% vs 80% required) because the commit-funnel logic added for the slider lag/flicker fix (drag/cancel/guard handling in PerpsOrderView and PerpsClosePositionView, plus the leverage confirm flush/clamp fix) had no direct test coverage. Adds targeted tests that drive the real onValueChange/onDragEnd/onTouchCancel wiring and assert on the resulting commits instead of only checking that components render. Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ 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 ab95373. Configure here.
🧪 Flaky unit test detectionRun history flaky detectionHistorical failure rate is a hint, not proof — review each suggestion in context. See the flaky-test-detection skill for the full pattern reference and manual audit workflow. Failures / runs sampled per window:
AI-detected flaky patterns
|
…lines Use toBeOnTheScreen() instead of toBeDefined() for element-presence assertions, and select the leverage row by its stable testID instead of the i18n-able "Leverage" label (Bugbot). Co-authored-by: Cursor <cursoragent@cursor.com>
🔍 Smart E2E Test Selection
click to see 🤖 AI reasoning detailsE2E Test Selection:
Tag selection rationale:
The changes are scoped to Perps UI components only - no changes to core controllers, navigation, or shared infrastructure that would affect other test areas. Performance Test Selection: |
|
⚡ Performance Test Results
✅ All tests passed · 2 tests · 1 device 📱 Devices tested (1)Android: Google Pixel 8 Pro (v14.0) ✅ Passed Tests (2)
Branch: |




fix(perps): eliminate trade screen slider lag and flicker
Description
The amount slider on the Perps trade screen (and the Close Position screen) was lagging and flickering while dragging, and would visibly "snap" to a different position on release (TAT-3543).
Root cause: the custom
PerpsSlidercomponent calledrunOnJS(updateValue)on every panonUpdateframe (up to 60+/sec while dragging). OnPerpsOrderView, that callback calledsetAmount, which updatesorderForm.amountinPerpsOrderContext— the sole context consumed by the ~2,900-linePerpsOrderView. Every drag frame therefore re-rendered the whole view and re-ranusePerpsOrderFees(whichawaitscalculateFees(),applyFeeDiscount(), andhandlePointsEstimation()— all RewardsController calls) plususePerpsEstimatedSlippageand other memoized calculations, flooding the JS thread far faster than it could keep up.PerpsClosePositionViewhad the identical problem viaclosePercentage. Separately,PerpsSliderbuilt its pan/tap gesture objects directly in the component body (nouseMemo), so every one of those re-renders also tore down and rebuilt the native gesture recognizer mid-drag, independently causing stutter and an inconsistent final position.Solution: rewrote
PerpsSlideras a thin wrapper around the MetaMask Design SystemSlider(@metamask/design-system-react-native), which already memoizes its gesture and exposes two callbacks —onValueChange(fired every drag tick, for cheap display-only updates) andonDragEnd(fired once on release, for expensive side effects). This is the same pattern already used byBatchSellReviewTokenRow.PerpsOrderViewandPerpsClosePositionViewnow split their slider-driven state into a cheap live-display value (updates every frame) and a committed value (updates once per drag, ononDragEnd) that continues to drive fees/rewards/slippage/validation.PerpsAdjustMarginViewonly feeds cheap synchronous math, so it needed no call-site changes — it benefits from the gesture-memoization fix for free.Update from
mainForward-merged
maininto this branch (merge commit, no rebase) to resolve conflicts created bymain's independent evolution of the oldPerpsSlidersince this branch forked: avariant?: 'default' | 'compact'prop and ashowPercentageMarkersprop (both now used by the newPerpsProOrderFormcompact slider), plus aprogressColor/quickValuesprop pair that no real caller used and was dropped. Both new props were ported onto the DS-wrapper rewrite above (variant="compact"maps totrackInset={0}+ a densetwClassName;showPercentageMarkersmaps to the DSSlider'sshowRangeDots, decoupled from the range-label prop), soPerpsProOrderForm,PerpsAdjustMarginView,PerpsOrderView, andPerpsClosePositionViewall keep working unchanged.Also bumped
@metamask/design-system-react-nativeto^0.38.1, which includes MetaMask/metamask-design-system#1397 — a separate, complementary fix for a stale controlled-value echo inuseSliderGesturethat caused the DSSlider's thumb to visibly rewind/flicker on rapid taps or fast pans.Changelog
CHANGELOG entry: Fixed the Perps trade and close-position amount sliders lagging, flickering, and jumping to a different value on release
Related issues
Refs: TAT-3543
Manual testing steps
Screenshots/Recordings
Before
N/A - recording not captured in this session (no device/simulator available). This is a runtime drag-responsiveness fix, best verified live per the manual testing steps above rather than a static screenshot; author to add a before recording prior to marking ready for review.
After
N/A - recording not captured in this session (no device/simulator available). Author to add an after recording prior to marking ready for review.
Pre-merge author checklist
Performance checks (if applicable)
usePerpsOrderFees/order submission is untouchedFor performance guidelines and tooling, see the Performance Guide.
Pre-merge reviewer checklist
Note
Medium Risk
Changes how Perps order and close sizes are committed at submit time (drag-end vs per-frame), which could affect edge cases around cancelled gestures and rapid confirm taps, though guards and new tests target those paths.
Overview
Fixes Perps trade and close-position amount sliders that lagged, flickered, and could snap on release because every drag frame updated committed state and re-ran fees, slippage, and validation across large views.
PerpsSlideris now a thin wrapper around@metamask/design-system-react-nativeSlider(bumped to ^0.38.1), forwardingonDragEndand haptics while dropping the old gesture/reanimated implementation andPerpsSlider.styles.ts.PerpsOrderViewandPerpsClosePositionViewsplit live drag display from committed amount/percentage:onValueChangeupdates UI only;onDragEnd(and a sharedcommitAmount/commitClosePercentagefunnel) updates context/state that drives fees and submission. Place order / confirm close flush an in-flight drag instead of submitting stale values; leverage confirm clamps against the live drag amount;onTouchCancelhandles cancelled gestures; keypad commits can skip USD string resync to preserve mid-typing values like2..Tests were expanded for the commit funnel;
PerpsSliderunit tests now assert DS prop mapping.Reviewed by Cursor Bugbot for commit 93ff064. Bugbot is set up for automated code reviews on this repo. Configure here.