Skip to content

fix: pressability issues new arch#51835

Open
hannojg wants to merge 13 commits into
react:mainfrom
hannojg:fix/pressability-issues-by-measuring-native-view-hierarchy
Open

fix: pressability issues new arch#51835
hannojg wants to merge 13 commits into
react:mainfrom
hannojg:fix/pressability-issues-by-measuring-native-view-hierarchy

Conversation

@hannojg

@hannojg hannojg commented Jun 5, 2025

Copy link
Copy Markdown
Contributor

Summary:

Addresses: #51621

Fixes press ability issues on new arch by adding a separate path for measuring, called:

  • measureAsyncOnUI

this method will call into the platform MountingManager implementations to measure using just the native view hierarchy.
This way the measurement should always be correct.

Note: the method for measuring is basically copied over from old arch code.

Changelog:

[GENERAL] [ADDED] Add HostInstance.measureAsyncOnUI(callback) for measuring host views from the native view hierarchy on the UI thread.

[GENERAL] [FIXED] Pressability issues on new arch

Test Plan:

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jun 5, 2025
@hannojg hannojg marked this pull request as ready for review June 5, 2025 11:28
@facebook-github-bot facebook-github-bot added the Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. label Jun 5, 2025

@Hardanish-Singh Hardanish-Singh left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

LGTM!

@klxiaoniu

Copy link
Copy Markdown

Any updates on this?

@oleksandr-dziuban

Copy link
Copy Markdown

Hello guys, can this fix be cherry-picked into v0.80.x, for example ?

@Dallas62

Dallas62 commented Aug 21, 2025

Copy link
Copy Markdown

Will also check if this resolve #53366 in next hours.

EDIT:
Maybe I'm testing it not the right way, but I always face this:
image

@SzyVsSi

SzyVsSi commented Aug 22, 2025

Copy link
Copy Markdown

Any alternative solutions while we wait for this PR to be merged?

@Bowlerr

Bowlerr commented Aug 28, 2025

Copy link
Copy Markdown

👀

@joacub

joacub commented Sep 5, 2025

Copy link
Copy Markdown

when is this going to be merged ?

@just-myself

Copy link
Copy Markdown

Any updates on this?

@hannojg

hannojg commented Oct 16, 2025

Copy link
Copy Markdown
Contributor Author

Let me update this PR today or tmrw so there are no merge conflicts and it works with the latest main, then we can maybe move it forward!

@hannojg hannojg force-pushed the fix/pressability-issues-by-measuring-native-view-hierarchy branch from de24f92 to 272dcfd Compare October 21, 2025 08:37
@react-native-bot

react-native-bot commented Oct 21, 2025

Copy link
Copy Markdown
Collaborator

Warnings
⚠️ ❗ JavaScript API change detected - This PR commits an update to ReactNativeApi.d.ts, indicating a change to React Native's public JavaScript API. Please include a clear changelog message. This change will be subject to extra review.

This change was flagged as: BREAKING

Generated by 🚫 dangerJS against 7f2df56

@hannojg

hannojg commented Oct 21, 2025

Copy link
Copy Markdown
Contributor Author

Hey, I rewrote this PR to be mergable against main.
The main change here was that we need to call through the NativeDOM module. I added the new measureAsyncOnUI method there, can we please double check whether this is the correct place for it?
Alternatively we could call to the fabric ui manager, but it seems to me we want to avoid that in NativeDOM ?

Kindly requesting another round of review @Hardanish-Singh
Will also ping maintainers on discord!


For folks needing this fix for older RN versions, I kept a copy of the old branch targeting RN 0.79 here:

@cipolleschi cipolleschi left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This change is kind of big and touches multiple parts of React Native.
I strongly suggest to wrap it within a feature flag so we can easily switch back to the old behavior in case it misbehave in prod in our apps.

ideally, it would be better if you can split it in smaller pieces, but perhaps it is easier to review as a whole for an approval and then we can split it.

I don't have the full picture on this, so I'll forward to somebody in the team that can provide a good review.

@hannojg

hannojg commented Oct 21, 2025

Copy link
Copy Markdown
Contributor Author

Thanks for the first quick check! Let me add a feature flag first. The feature flag could gate this call in

packages/react-native/Libraries/Pressability/Pressability.js:

    if (typeof this._responderID === 'number') {
      UIManager.measure(this._responderID, this._measureCallback);
    } else {
-      this._responderID.measure(this._measureCallback);
+      this._responderID.measureAsyncOnUI(this._measureCallback);
    }
  }

Putting the native code behind a feature flag would be a bit difficult since we touch turbo module specs.

Adding a new virtual method to the scheduler kind of trickles down into all those places touched here, so it would be a bit hard to split the PR up otherwise.

sethkfman added a commit to MetaMask/metamask-mobile that referenced this pull request Oct 22, 2025
<!--
Please submit this PR as a draft initially.
Do not mark it as "Ready for review" until the template has been
completely filled out, and PR status checks have passed at least once.
-->

## **Description**

Ported patch from react/react-native#51835 to
version 0.76.9

I also had to enable build from source for Android in order to make
patch work, that will result in increased build time.

## **Changelog**

<!--
If this PR is not End-User-Facing and should not show up in the
CHANGELOG, you can choose to either:
1. Write `CHANGELOG entry: null`
2. Label with `no-changelog`

If this PR is End-User-Facing, please write a short User-Facing
description in the past tense like:
`CHANGELOG entry: Added a new tab for users to see their NFTs`
`CHANGELOG entry: Fixed a bug that was causing some NFTs to flicker`

(This helps the Release Engineer do their job more quickly and
accurately)
-->

CHANGELOG entry:

## **Related issues**

Fixes:

## **Manual testing steps**

```gherkin
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**

<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->

### **Before**

<!-- [screenshots/recordings] -->

### **After**

<!-- [screenshots/recordings] -->

## **Pre-merge author checklist**

- [ ] I’ve followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile
Coding
Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md).
- [ ] I've completed the PR template to the best of my ability
- [ ] I’ve included tests if applicable
- [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [ ] I’ve applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)).
Not required for external contributors.

## **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.

<!-- CURSOR_SUMMARY -->
---

> [!NOTE]
> Introduces a React Native patch adding `measureAsyncOnUI` across
Fabric/UIManager and switches app components to plain
`TouchableOpacity`, while enabling Android builds to compile RN from
source.
> 
> - **React Native (patched)**:
> - Add `measureAsyncOnUI` API across Fabric/UIManager (JS, iOS,
Android, JNI) and expose via `UIManagerBinding`.
> - Update `Pressability` and `ReactFabricHostComponent` to use
`measureAsyncOnUI` for view measurement.
> - **Android Build**:
> - Enable building `react-native` from source with Gradle dependency
substitutions.
> - **App Components**:
> - Simplify touch handling in `ButtonBase`, `ListItemMultiSelect`, and
`ListItemSelect` by removing `react-native-gesture-handler` tap wrappers
and using `RNTouchableOpacity` directly.
> - **Tooling**:
> - Add Yarn patch/resolution for `react-native@0.76.9` and update
`yarn.lock`.
> 
> <sup>Written by [Cursor
Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit
da7a9a0. This will update automatically
on new commits. Configure
[here](https://cursor.com/dashboard?tab=bugbot).</sup>
<!-- /CURSOR_SUMMARY -->
@Dallas62

Copy link
Copy Markdown

Hi @cipolleschi,

It would be great if someone from Meta could take a look to all pressability issues in new arch or try to implement the requested feature flag.

It's a bug present and reported multiples time from months now...

@lodev09

lodev09 commented Jan 1, 2026

Copy link
Copy Markdown

The Pressable onPress issue is breaking TrueSheet nesting capabilities 😢. Hopefully this gets merged soon! 🙏

MelvinBot added a commit to Expensify/App that referenced this pull request Mar 19, 2026
Add react-native patch based on upstream PR react/react-native#51835
to fix Android-specific onPress failures for Pressable components
inside Tooltip on certain Samsung devices.

The root cause is that Pressability.measure() reads stale layout info
from the shadow tree while Reanimated has already updated the native
view. This patch introduces measureAsyncOnUI which measures using the
native view hierarchy on the UI thread.

Also removes the createPressHandler workaround since this patch
properly fixes the root cause.

Co-authored-by: Linh Vo <linhvovan29546@users.noreply.github.com>
MelvinBot added a commit to Expensify/App that referenced this pull request Mar 24, 2026
Add react-native patch based on upstream PR react/react-native#51835
to fix Android-specific issue where onPress events do not trigger for
Pressable components when used inside a Tooltip on certain Samsung devices.

Also removes the createPressHandler workaround since this patch properly
fixes the root cause.

Co-authored-by: Linh Vo <linhvovan29546@users.noreply.github.com>
MelvinBot added a commit to Expensify/App that referenced this pull request Mar 31, 2026
Re-apply the changes from PR #86160 which was reverted. Adds a
react-native patch based on upstream react/react-native#51835
to fix onPress events not triggering for Pressable components
inside Tooltips on certain Samsung devices.

Also removes the createPressHandler workaround since this patch
properly fixes the root cause.

Co-authored-by: linhvovan29546 <linhvovan29546@gmail.com>
Co-authored-by: Linh Vo <linhvovan29546@users.noreply.github.com>
MelvinBot added a commit to Expensify/App that referenced this pull request Mar 31, 2026
Re-apply the changes from PR #86160 which was reverted. Adds a
react-native patch based on upstream react/react-native#51835
to fix onPress events not triggering for Pressable components
inside Tooltips on certain Samsung devices.

Also removes the createPressHandler workaround since this patch
properly fixes the root cause.
@hannojg hannojg force-pushed the fix/pressability-issues-by-measuring-native-view-hierarchy branch from 7f2df56 to 7aa9495 Compare June 18, 2026 07:28
@github-actions

Copy link
Copy Markdown

Warning

JavaScript API change detected

This PR commits an update to ReactNativeApi.d.ts, indicating a change to React Native's public JavaScript API.

  • Please include a clear changelog message.
  • This change will be subject to additional review.

This change was flagged as: POTENTIALLY_BREAKING

@hannojg

hannojg commented Jun 18, 2026

Copy link
Copy Markdown
Contributor Author

@cipolleschi

  • I updated this PR to be mergeable against main
  • I hide this behind a new feature flag, shouldPressabilityUseNativeViewHierarchyForMeasurement which by default is off
  • I think its a bit difficult to split up, except maybe for feature flag introduction VS actual code changes, let me know if thats preferred!

Would appreciate it very much if we could revisit this PR, thanks in advance! 😊

@hannojg hannojg requested a review from cipolleschi June 18, 2026 08:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team.

Projects

None yet

Development

Successfully merging this pull request may close these issues.