Skip to content

Conversation

@michalconsensys
Copy link
Contributor

@michalconsensys michalconsensys commented Oct 24, 2025

Description

Fixed a bug in the leverage slider where tapping or long pressing on the slider track did not trigger the onDragEnd callback, causing inconsistent state management between tap and drag interactions.

Changelog

CHANGELOG entry: Fixed leverage slider tap and long press gesture not finalizing state updates correctly

Related issues

Fixes: https://consensyssoftware.atlassian.net/browse/TAT-1826

Manual testing steps

Feature: Leverage Slider Tap Gesture

  Scenario: user taps on leverage slider track
    Given user is on the perps market details screen
    And user taps "Long" or "Short" button
    And leverage adjustment modal is displayed

    When user taps anywhere on the leverage slider track
    Then the leverage value updates immediately
    And the dragging state resets to false
    And the liquidation price recalculates

  Scenario: user holds the slider for a long time
    Given user is on the leverage adjustment modal
    
    When user holds the slider
    Then releasing the slider properly finalizes the leverage value
    And the liquidation price updates when user releases the slider
    And no stale state remains between taps

Screenshots/Recordings

Before

Simulator.Screen.Recording.-.iPhone.16e.-.2025-10-27.at.07.12.51.mov

After

Simulator.Screen.Recording.-.iPhone.16e.-.2025-10-27.at.07.09.55.mov

Pre-merge author checklist

Pre-merge reviewer checklist

  • I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed).
  • I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.

Note

Ensures tap and long-press finalize leverage changes, adds comprehensive gesture tests, and introduces a non-flickering skeleton state for liquidation data.

  • PerpsLeverageBottomSheet:
    • Gesture handling:
      • Make onDragStart/onDragEnd required and invoke unconditionally during pan end/begin.
      • Add handleHoldEnd and wire to Gesture.Tap and Gesture.LongPress; compose with Pan via Gesture.Simultaneous.
      • Add testID leverage-slider-track for gesture/layout targeting.
    • Loading/Skeleton:
      • Introduce shouldShowSkeleton to control liquidation UI and prevent flicker; reset on close and when value unchanged.
      • Use Number.parseFloat; set dynamicLiquidationPrice to 0 while isCalculating.
      • Trigger skeleton on slider drag updates and on quick-select changes.
  • Tests (PerpsLeverageBottomSheet.test.tsx):
    • Add gesture utilities/mocks (fireGestureHandler, LongPress, haptics) and extensive tests for pan/tap/long-press flows, state updates, and haptic integration.
    • Add coverage for quick-select behavior, risk styling, liquidation formatting, skeleton visibility, and layout+gesture sequences.

Written by Cursor Bugbot for commit eeb9bc7. This will update automatically on new commits. Configure here.

@github-actions
Copy link
Contributor

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.

@github-actions github-actions bot added size-S and removed size-XS labels Oct 27, 2025
@michalconsensys michalconsensys added the team-perps Perps team label Oct 27, 2025
@michalconsensys michalconsensys marked this pull request as ready for review October 27, 2025 08:13
@michalconsensys michalconsensys requested a review from a team as a code owner October 27, 2025 08:13
@michalconsensys michalconsensys requested a review from a team as a code owner October 27, 2025 08:42
@github-actions github-actions bot added size-M and removed size-S labels Oct 27, 2025
@github-actions github-actions bot added size-L and removed size-M labels Oct 27, 2025
@codecov-commenter
Copy link

Codecov Report

❌ Patch coverage is 66.66667% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 77.15%. Comparing base (a00ab1a) to head (81c7525).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
...psLeverageBottomSheet/PerpsLeverageBottomSheet.tsx 66.66% 2 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main   #21608   +/-   ##
=======================================
  Coverage   77.15%   77.15%           
=======================================
  Files        3720     3720           
  Lines       93544    93548    +4     
  Branches    17947    17949    +2     
=======================================
+ Hits        72171    72179    +8     
- Misses      16475    16476    +1     
+ Partials     4898     4893    -5     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@github-actions github-actions bot added size-XL and removed size-L labels Oct 27, 2025
@github-actions github-actions bot added size-L and removed size-XL labels Oct 27, 2025
@gambinish
Copy link
Contributor

gambinish commented Oct 27, 2025

Pulled down the branch, and this is the behavior that I am seeing:

Screen.Recording.2025-10-27.at.8.50.59.AM.mov

It seems like the Liquidation Price briefly gets computed with the "estimated" value before entering into the loading state for the computed value.

I think this is partly what we are trying to solve in this bugfix. I wonder if we can trigger the loading skeleton onDrag to avoid rendering the theoretical value entirely. I think that MSO is aligned with omitting the this estimated value entirely since it can be confusing for the user

Copy link
Contributor

@abretonc7s abretonc7s left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems like a very large amount of tests added, could they be grouped with better assertions?

cursor[bot]

This comment was marked as outdated.

cursor[bot]

This comment was marked as outdated.

@github-actions github-actions bot added size-L and removed size-M labels Oct 28, 2025
cursor[bot]

This comment was marked as outdated.

cursor[bot]

This comment was marked as outdated.

cursor[bot]

This comment was marked as outdated.

@michalconsensys
Copy link
Contributor Author

@abretonc7s @gambinish I've updated the tests and made it to display a loading indicator when the user starts dragging the slider, however can't quite get the coverage above 80%. Could you please have a look again? Thanks!

cursor[bot]

This comment was marked as outdated.

@github-actions github-actions bot added size-M and removed size-L labels Oct 29, 2025
@github-actions github-actions bot added size-L and removed size-M labels Oct 30, 2025
@sonarqubecloud
Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
78.8% Coverage on New Code (required ≥ 80%)

See analysis details on SonarQube Cloud

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants