Skip to content

fix: problematic dismissable ref#31658

Open
gambinish wants to merge 1 commit into
mainfrom
fix/tpsl-defocus-bug
Open

fix: problematic dismissable ref#31658
gambinish wants to merge 1 commit into
mainfrom
fix/tpsl-defocus-bug

Conversation

@gambinish

@gambinish gambinish commented Jun 12, 2026

Copy link
Copy Markdown
Member

Description

Problem

When editing a position's TP/SL and tapping directly from the price field to the % gain/loss field, typing a percentage did not auto-fill the trigger price and the Set button stayed disabled. iOS-only bug.

Root cause

The iOS keyboard-dismiss fix introduced a boolean guard (isProgrammaticDismissRef) that was set synchronously on every input focus. On iOS, the new field's onFocus fires before the old field's onBlur, so the guard was active when the price field's blur arrived — suppressing it and leaving tpPriceInputFocused stuck true. The percentage→price auto-fill was gated on !tpPriceInputFocused, so it never ran.

Fix

Scoped the guard to a specific input — replaced the boolean ref with a string | null ref that stores the id of the field being dismissed. Only that field's programmatic blur is suppressed; all other fields' genuine blurs (including the previously-focused price field) are delivered normally and correctly clear focus state.

Hardened the hook — removed the !tpPriceInputFocused / !slPriceInputFocused gate from the percentage change handlers entirely. When the user is typing a percentage, that field is by definition the active source of truth and the trigger price must always be computed, regardless of stale focus flags.

Tests

Added a regression test for the iOS focus/blur race (focus % before blur on price, no timer flush) verifying the price field's blur handler still fires.

Updated and added hook-level tests confirming percentage→price auto-fill works even when a price field's focus flag is stale.

Changelog

CHANGELOG entry:

Related issues

Fixes:

Manual testing steps

Feature: my feature name

  Scenario: user [verb for user action]
    Given [describe expected initial app state]

    When user [verb for user action]
    Then [describe expected outcome]

Screenshots/Recordings

Android:

Screen.Recording.2026-06-12.at.11.02.36.AM.mov

iOS:
https://github.com/user-attachments/assets/20902575-80bf-4a70-bf9b-d5c8a2d10bb1

Pre-merge author checklist

Performance checks (if applicable)

  • I've tested on Android
    • Ideally on a mid-range device; emulator is acceptable
  • I've tested with a power user scenario
    • Use these power-user SRPs to import wallets with many accounts and tokens
  • I've instrumented key operations with Sentry traces for production performance metrics

For performance guidelines and tooling, see the Performance Guide.

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.

@mm-token-exchange-service mm-token-exchange-service Bot added the team-perps Perps team label Jun 12, 2026
@mm-token-exchange-service

mm-token-exchange-service Bot commented Jun 12, 2026

Copy link
Copy Markdown

PR template — items to address before "Ready for review"

Blocking — these items fail the workflow until fixed:

  • Changelog section has an empty CHANGELOG entry: line. Fill in a description, write CHANGELOG entry: null, or apply the no-changelog label.

Warnings — informational, address before merging:

  • Related issues section is empty. Add Fixes: #123 / Closes: <URL> / Refs: <Jira key>, or write a short rationale after the colon.
  • Manual testing steps still contain template content (the Gherkin example title or a [...] placeholder). Replace with real steps, or write N/A — <reason>.
  • Pre-merge author checklist has unchecked items (e.g. "I've followed MetaMask Contributor Docs and MetaMask Mobile Coding Standards."). Every box must be consciously checked — see docs/readme/ready-for-review.md.

See docs/readme/ready-for-review.md for the full Definition of Ready for Review.

@github-actions

Copy link
Copy Markdown
Contributor

🔍 Smart E2E Test Selection

  • Selected E2E tags: SmokePerps, SmokeWalletPlatform
  • Selected Performance tags: None (no tests recommended)
  • Risk Level: medium
  • AI Confidence: 88%
click to see 🤖 AI reasoning details

E2E Test Selection:
The PR contains targeted bug fixes for the Perps TP/SL (Take Profit/Stop Loss) view and its associated hook:

  1. PerpsTPSLView.tsx: Fixed an iOS focus/blur race condition by replacing a single boolean isProgrammaticDismissRef with a field-specific programmaticDismissInputRef that tracks which exact input is being programmatically dismissed. This ensures that when a user moves focus from one field to another, only the programmatically-dismissed field's blur is suppressed — other fields' genuine blurs still fire correctly.

  2. usePerpsTPSLForm.ts: Removed the !tpPriceInputFocused and !slPriceInputFocused guards from the percentage-to-price auto-fill logic. Previously, if focus state was stale (due to iOS focus/blur ordering), typing a percentage would silently fail to update the trigger price. Now the price is always computed when the user types a percentage.

  3. Test files: Added regression tests covering the iOS focus/blur race scenario and updated existing tests to reflect the new behavior.

Tag Selection Rationale:

  • SmokePerps: Directly tests the Perps functionality that was modified (TP/SL form, Add Funds flow, balance verification). This is the primary tag for validating these changes.
  • SmokeWalletPlatform: Per the SmokePerps tag description, Perps is a section inside the Trending tab (SmokeWalletPlatform). Changes to Perps views affect Trending, so this tag is required as a dependency.

Tags NOT selected:

  • SmokeConfirmations: The TP/SL view itself doesn't involve transaction confirmations — it's a form for setting order parameters. The SmokePerps→SmokeConfirmations dependency applies to the Add Funds flow (on-chain deposits), not the TP/SL form changes specifically.
  • All other tags: No shared infrastructure, navigation, core controllers, or other feature areas are affected by these changes.

Risk Assessment: Medium — the changes fix a real iOS-specific bug in focus/blur handling that could affect user input behavior on iOS devices. The fix is well-scoped and accompanied by unit tests, but the iOS focus/blur race condition is subtle and worth E2E validation.

Performance Test Selection:
The changes are bug fixes for iOS focus/blur race conditions in the Perps TP/SL form. No performance-sensitive code paths are modified — no rendering loops, no data fetching, no state management at scale. The @PerformancePreps tag covers perps market loading and position management, but these changes only affect input focus/blur handling logic which has no measurable performance impact.

View GitHub Actions results

@gambinish gambinish marked this pull request as ready for review June 12, 2026 21:11
@gambinish gambinish requested a review from a team as a code owner June 12, 2026 21:11
@github-actions github-actions Bot added the risk:medium AI analysis: medium risk label Jun 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

risk:medium AI analysis: medium risk size-M team-perps Perps team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants