Skip to content

fix(social-ai): make back from notification-driven trader position behave naturally#30052

Open
xavier-brochard wants to merge 3 commits into
mainfrom
fix/trader-position-back-animation
Open

fix(social-ai): make back from notification-driven trader position behave naturally#30052
xavier-brochard wants to merge 3 commits into
mainfrom
fix/trader-position-back-animation

Conversation

@xavier-brochard
Copy link
Copy Markdown
Contributor

@xavier-brochard xavier-brochard commented May 12, 2026

Description

Watch this =======> https://www.loom.com/share/c981e1ec4ca245b48d68759e5fbfe34e

When a user opened TraderPositionView from a notification (in-app or push), pressing back used to slide TraderProfileView in from the right — reading as a forward navigation. The cause was handleBack falling back to navigation.replace(PROFILE) when the deeplink stack didn't already contain a Profile underneath.

This PR removes that fallback and the assumption that "back from a notification-driven position should land on Profile". Back now does the natural and conventional thing — navigation.goBack() — and returns the user to wherever they were before opening the position:

Entry point Back lands on
Cold-start push notification Wallet Home (the app's initial route)
In-app notification panel tap Notifications panel
In-app row tap from Profile Profile (unchanged)

The trader's name in the position header will receive a link on a follow up PR to ensure navigation from position > profile is available in every case.

handleSocialTraderPositionUrl is correspondingly simplified back to a single navigate(POSITION) call (no WalletHome/Profile pre-population).

Also: assign the two social deeplink handlers (handleSocialLeaderboardUrl.ts, handleSocialTraderPositionUrl.ts) to @MetaMask/social-ai in .github/CODEOWNERS.

Changelog

CHANGELOG entry: Fixed a confusing forward-style animation when pressing back from a trader position opened via a notification.

Related issues

Refs: notification-driven deeplink UX

Manual testing steps

You can simulate the deeplink on the iOS simulator with xcrun simctl openurl (substitute real positionId / traderId):

# Cold-start push: terminate the app first, then open the URL
xcrun simctl terminate booted io.metamask.MetaMask
xcrun simctl openurl booted "metamask://social-trader-position?positionId=<positionId>&traderId=<traderId>&deduplication_id=dedup-1&notification_event=follow_newtrade_buy"

# In-app entry: open the URL while the app is foregrounded (optionally open
# the in-app Notifications panel first to verify back returns there)
xcrun simctl openurl booted "metamask://social-trader-position?positionId=<positionId>&traderId=<traderId>"
Feature: Back navigation from a notification-driven trader position

  Scenario: Cold-start push notification
    Given the app is not running
    When the user taps a followed-trader trade push notification
    Then the app opens directly on TraderPositionView
    When the user taps the back button (or performs an iOS edge-swipe from the left)
    Then TraderPositionView animates off to the right
    And the user lands on Wallet Home

  Scenario: In-app notification panel tap
    Given the user has the in-app Notifications panel open
    When the user taps a followed-trader trade notification row
    Then the app navigates to TraderPositionView
    When the user taps the back button
    Then TraderPositionView animates off to the right
    And the user lands back on the Notifications panel

  Scenario: Row tap from the trader's Profile (existing in-app flow, unchanged)
    Given the user is viewing a trader's Profile
    When the user taps one of the trader's positions
    Then the app navigates to TraderPositionView
    When the user taps the back button
    Then TraderPositionView animates off to the right
    And the user lands back on the trader's Profile

Screenshots/Recordings

Before

N/A

After

https://www.loom.com/share/c981e1ec4ca245b48d68759e5fbfe34e

Pre-merge author checklist

Performance checks (if applicable)

  • I've tested on Android
  • I've tested with a power user scenario
  • I've instrumented key operations with Sentry traces for production performance metrics

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

Medium Risk
Adjusts navigation behavior for TraderPositionView and social deeplinks, which can affect user flow and back-stack correctness across cold-start and in-app notification entry points.

Overview
Pressing back from TraderPositionView now always uses navigation.goBack() (removing the prior deeplink-specific replace(PROFILE) fallback) to avoid a forward-style transition and return users to their actual prior screen.

Social deeplink handling for trader positions is simplified to a single navigate(POSITION) flow (tests updated to match), and .github/CODEOWNERS assigns the legacy social deeplink handlers to @MetaMask/social-ai.

Reviewed by Cursor Bugbot for commit 26d7ab3. Bugbot is set up for automated code reviews on this repo. Configure here.

@metamaskbotv2 metamaskbotv2 Bot added the team-social-ai Social & AI team label May 12, 2026
@github-actions github-actions Bot added the pr-not-ready-for-e2e Skip E2E and block merging. Remove this label once the PR is ready to run the E2E tests. label May 12, 2026
@github-actions
Copy link
Copy Markdown
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.

@xavier-brochard xavier-brochard marked this pull request as ready for review May 12, 2026 16:16
@xavier-brochard xavier-brochard requested a review from a team as a code owner May 12, 2026 16:16
@xavier-brochard xavier-brochard removed the pr-not-ready-for-e2e Skip E2E and block merging. Remove this label once the PR is ready to run the E2E tests. label May 12, 2026
Copy link
Copy Markdown
Contributor

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ 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 d10b246. Configure here.

Comment thread app/core/DeeplinkManager/handlers/legacy/handleSocialTraderPositionUrl.ts Outdated
@xavier-brochard xavier-brochard enabled auto-merge May 12, 2026 16:33
Bigshmow
Bigshmow previously approved these changes May 12, 2026
@xavier-brochard xavier-brochard marked this pull request as draft May 13, 2026 06:43
auto-merge was automatically disabled May 13, 2026 06:43

Pull request was converted to draft

@xavier-brochard xavier-brochard marked this pull request as ready for review May 13, 2026 07:11
@xavier-brochard xavier-brochard enabled auto-merge May 13, 2026 07:14
zone-live
zone-live previously approved these changes May 13, 2026
@xavier-brochard xavier-brochard dismissed stale reviews from zone-live and Bigshmow via dd5a494 May 15, 2026 15:35
@xavier-brochard xavier-brochard force-pushed the fix/trader-position-back-animation branch 2 times, most recently from dd5a494 to 5d47160 Compare May 15, 2026 15:36
…have naturally

When a TraderPositionView was opened from a notification, pressing back used
to slide TraderProfileView in from the right (a forward animation), because
handleBack fell back to navigation.replace.

Replace that fallback with a plain navigation.goBack() and stop trying to
synthesize a Profile underneath in the deeplink handler. Back now returns
to wherever the user was before opening the position:
- cold-start push:                Wallet Home
- in-app notification panel tap:  Notifications panel
- in-app row tap from Profile:    Profile (unchanged)

The trader's name in the position header remains the affordance for
navigating onward to the trader's Profile.

Also: assign the two social deeplink handlers to @MetaMask/social-ai in
CODEOWNERS.

Co-authored-by: Cursor <cursoragent@cursor.com>
@xavier-brochard xavier-brochard force-pushed the fix/trader-position-back-animation branch from 5d47160 to 6abfeee Compare May 15, 2026 15:47
@xavier-brochard xavier-brochard changed the title fix(social-ai): natural back animation from notification-driven trader position fix(social-ai): make back from notification-driven trader position behave naturally May 15, 2026
@xavier-brochard xavier-brochard added the pr-not-ready-for-e2e Skip E2E and block merging. Remove this label once the PR is ready to run the E2E tests. label May 15, 2026
@xavier-brochard xavier-brochard marked this pull request as draft May 15, 2026 15:58
auto-merge was automatically disabled May 15, 2026 15:58

Pull request was converted to draft

…cialTraderPositionUrl

This commit reorganizes the import statements in `handleSocialTraderPositionUrl.ts` for better clarity and consistency. It also removes outdated comments regarding navigation behavior, streamlining the code for future maintenance.
@xavier-brochard xavier-brochard marked this pull request as ready for review May 15, 2026 16:15
@xavier-brochard xavier-brochard removed the pr-not-ready-for-e2e Skip E2E and block merging. Remove this label once the PR is ready to run the E2E tests. label May 15, 2026
@xavier-brochard xavier-brochard enabled auto-merge May 15, 2026 16:25
@github-actions
Copy link
Copy Markdown
Contributor

🔍 Smart E2E Test Selection

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

E2E Test Selection:
The PR makes the following changes:

  1. TraderPositionView.tsx: Simplifies the handleBack function to always call navigation.goBack() instead of checking navigation state and conditionally replacing the route with the Profile screen. This is a UX/navigation behavior change within the Social Leaderboard feature.

  2. TraderProfileView.tsx: Import reordering only — purely cosmetic, no functional change.

  3. handleSocialTraderPositionUrl.ts: Import reordering only — purely cosmetic, no functional change.

  4. Test files: Updated to reflect the simplified back navigation logic (removed mockReplace and mockGetState, simplified test cases).

  5. .github/CODEOWNERS: Added ownership entries for social leaderboard deeplink handlers.

The Social Leaderboard (TraderPositionView, TraderProfileView) is part of the Trending/discovery feature set. No dedicated E2E smoke tests exist for Social Leaderboard specifically. The closest applicable tag is SmokeWalletPlatform, which covers the Trending tab and its subsections (including navigation integration). The changes are low-risk as they only affect back navigation behavior within a feature-specific screen, with no impact on shared components, core infrastructure, or other user flows.

Performance Test Selection:
The changes are limited to navigation logic simplification (goBack instead of conditional replace) and import reordering. There are no UI rendering changes, list component changes, data loading changes, or performance-sensitive code paths affected. No performance tests are warranted.

View GitHub Actions results

@sonarqubecloud
Copy link
Copy Markdown

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

Labels

size-M team-social-ai Social & AI team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants