Skip to content

fix: Updated ButtonBase and ListItems to have gesture detection on Android cp-7.53.0#18104

Merged
brianacnguyen merged 22 commits into
mainfrom
fix/buttons
Aug 15, 2025
Merged

fix: Updated ButtonBase and ListItems to have gesture detection on Android cp-7.53.0#18104
brianacnguyen merged 22 commits into
mainfrom
fix/buttons

Conversation

@brianacnguyen

@brianacnguyen brianacnguyen commented Aug 7, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor

Description

This PR updated the ButtonBase, ListItemSelect, and ListItemMultiSelect to include Gesture Detector, resolving a current bug in react-native's TouchableOpacity component occasionally not triggering onPress on Android

Changelog

CHANGELOG entry:

Related issues

Fixes: #18147

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

Before

After

Everything still functional
https://github.com/user-attachments/assets/f4e9f41e-34f7-4bf8-a090-860bee411050
https://github.com/user-attachments/assets/f2791eb2-9520-4ce7-ad3e-6d0933d59c35

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.

@brianacnguyen brianacnguyen requested a review from a team as a code owner August 7, 2025 20:32
@brianacnguyen brianacnguyen self-assigned this Aug 7, 2025
@github-actions

github-actions Bot commented Aug 7, 2025

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.

@brianacnguyen brianacnguyen added team-design-system All issues relating to design system in Mobile Run Smoke E2E No QA Needed Apply this label when your PR does not need any QA effort. labels Aug 7, 2025
@github-actions

github-actions Bot commented Aug 7, 2025 •
edited by metamaskbot
Loading

Copy link
Copy Markdown
Contributor

https://bitrise.io/ Bitrise

❌❌❌ pr_smoke_e2e_pipeline failed on Bitrise! ❌❌❌

Commit hash: 2ef8a6d
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/5a452761-9e01-4abc-af40-df9afb6df15d

Note

  • You can rerun any failed steps by opening the Bitrise build, tapping Rebuild on the upper right then Rebuild unsuccessful Workflows
  • You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

Tip

  • Check the documentation if you have any doubts on how to understand the failure on bitrise

cursor[bot]

This comment was marked as outdated.

Sign in to view

cursor[bot]

This comment was marked as outdated.

Sign in to view

@brianacnguyen brianacnguyen requested a review from a team as a code owner August 8, 2025 15:31
@github-actions

github-actions Bot commented Aug 8, 2025 •
edited by metamaskbot
Loading

Copy link
Copy Markdown
Contributor

https://bitrise.io/ Bitrise

❌❌❌ pr_smoke_e2e_pipeline failed on Bitrise! ❌❌❌

Commit hash: 3cbff92
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/b194a4e5-5470-4f30-94f9-56e84fca3361

Note

  • You can rerun any failed steps by opening the Bitrise build, tapping Rebuild on the upper right then Rebuild unsuccessful Workflows
  • You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

Tip

  • Check the documentation if you have any doubts on how to understand the failure on bitrise

cursor[bot]

This comment was marked as outdated.

Sign in to view

@github-actions

github-actions Bot commented Aug 8, 2025 •
edited by metamaskbot
Loading

Copy link
Copy Markdown
Contributor

https://bitrise.io/ Bitrise

❌❌❌ pr_smoke_e2e_pipeline failed on Bitrise! ❌❌❌

Commit hash: bcf7ec6
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/8486da82-5386-4cf6-bddc-0741dd594053

Note

  • You can rerun any failed steps by opening the Bitrise build, tapping Rebuild on the upper right then Rebuild unsuccessful Workflows
  • You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

Tip

  • Check the documentation if you have any doubts on how to understand the failure on bitrise

@brianacnguyen brianacnguyen changed the title fix: Updated ButtonBase and ListItems to have gesture detection fix: Updated ButtonBase and ListItems to have gesture detection cp-7.53.0 Aug 8, 2025
@brianacnguyen brianacnguyen changed the title fix: Updated ButtonBase and ListItems to have gesture detection cp-7.53.0 fix: Updated ButtonBase and ListItems to have gesture detection on Android cp-7.53.0 Aug 8, 2025
cursor[bot]

This comment was marked as outdated.

Sign in to view

cursor[bot]

This comment was marked as outdated.

Sign in to view

cursor[bot]

This comment was marked as outdated.

Sign in to view

@github-actions

github-actions Bot commented Aug 8, 2025

Copy link
Copy Markdown
Contributor

https://bitrise.io/ Bitrise

🔄🔄🔄 pr_smoke_e2e_pipeline started on Bitrise...🔄🔄🔄

Commit hash: a30c9fe
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/253bc654-72a4-44ce-84bc-43da35933bfb

Note

  • This comment will auto-update when build completes
  • You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

cursor[bot]

This comment was marked as outdated.

Sign in to view

cursor[bot]

This comment was marked as outdated.

Sign in to view

@brianacnguyen brianacnguyen added skip-sonar-cloud Only used for bypassing sonar cloud when failures are not relevant to the changes. Run Smoke E2E and removed Run Smoke E2E labels Aug 14, 2025
@github-actions

github-actions Bot commented Aug 14, 2025 •
edited by metamaskbot
Loading

Copy link
Copy Markdown
Contributor

https://bitrise.io/ Bitrise

❌❌❌ pr_smoke_e2e_pipeline failed on Bitrise! ❌❌❌

Commit hash: c500a79
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/3c0acca6-6cf4-42fa-bd94-b37dd16ed2c5

Note

  • You can rerun any failed steps by opening the Bitrise build, tapping Rebuild on the upper right then Rebuild unsuccessful Workflows
  • You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

Tip

  • Check the documentation if you have any doubts on how to understand the failure on bitrise

@codecov-commenter

codecov-commenter commented Aug 14, 2025 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 25.00000% with 42 lines in your changes missing coverage. Please review.
✅ Project coverage is 75.62%. Comparing base (06f574b) to head (253625a).
⚠️ Report is 25 commits behind head on main.

Files with missing lines Patch % Lines
...uttons/Button/foundation/ButtonBase/ButtonBase.tsx 22.22% 12 Missing and 2 partials ⚠️
...s/List/ListItemMultiSelect/ListItemMultiSelect.tsx 30.00% 12 Missing and 2 partials ⚠️
.../components/List/ListItemSelect/ListItemSelect.tsx 22.22% 12 Missing and 2 partials ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##             main   #18104      +/-   ##
==========================================
+ Coverage   75.56%   75.62%   +0.06%     
==========================================
  Files        3059     3065       +6     
  Lines       70200    70615     +415     
  Branches    12067    12173     +106     
==========================================
+ Hits        53048    53405     +357     
- Misses      13752    13808      +56     
- Partials     3400     3402       +2

☔ 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.

vinnyhoward
vinnyhoward reviewed Aug 14, 2025
View reviewed changes
Comment thread app/component-library/components/List/ListItemSelect/ListItemSelect.tsx
const { styles } = useStyles(styleSheet, { style, isDisabled });

// Disable gesture wrapper in test environments to prevent test interference
const isE2ETest =

@vinnyhoward vinnyhoward Aug 14, 2025

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.

nit: I see this in 3 other places. We could possibly refactor this into a helper function?

vinnyhoward
vinnyhoward reviewed Aug 14, 2025
View reviewed changes
Comment thread app/component-library/components/List/ListItemSelect/ListItemSelect.tsx
import { DEFAULT_SELECTITEM_GAP } from './ListItemSelect.constants';
import { Gesture, GestureDetector } from 'react-native-gesture-handler';

const TouchableOpacity = ({

@vinnyhoward vinnyhoward Aug 14, 2025

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.

I'm possibly not looking hard enough but is there a difference with the new TouchableOpacity in the other 2-3 files? Should this be its own component?

vinnyhoward
vinnyhoward approved these changes Aug 14, 2025
View reviewed changes

@vinnyhoward vinnyhoward 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.

Approving and aware that this is temporary so there isn't need for optimization at the moment after speaking with Brian

@sonarqubecloud

sonarqubecloud Bot commented Aug 15, 2025

Copy link
Copy Markdown

Quality Gate Failed Quality Gate failed

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

See analysis details on SonarQube Cloud

@brianacnguyen brianacnguyen added this pull request to the merge queue Aug 15, 2025
auto-merge was automatically disabled August 15, 2025 15:51

Pull Request is not mergeable

Merged via the queue into main with commit 578bf72 Aug 15, 2025
51 of 54 checks passed
@brianacnguyen brianacnguyen deleted the fix/buttons branch August 15, 2025 15:56
@github-actions github-actions Bot locked and limited conversation to collaborators Aug 15, 2025
@metamaskbot metamaskbot added the release-7.54.0 Issue or pull request that will be included in release 7.54.0 label Aug 15, 2025
ieow
ieow reviewed Aug 25, 2025
View reviewed changes
Comment thread app/component-library/components/Buttons/Button/foundation/ButtonBase/ButtonBase.tsx
/* no-op for synthetic event */
},
} as GestureResponderEvent;
onPress(syntheticEvent);

@ieow ieow Aug 25, 2025

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.

would this onPress being called twice?
as onPress on the RNTouchableOpacity will be called as well?

https://github.com/MetaMask/metamask-mobile/pull/18104/files#diff-d8ff46ee74cd57f89c7ac7deb9144c994a7f9db5145060058638387669a72b5aR72

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

Reviewers

@ieow ieow ieow left review comments

@cursor cursor[bot] cursor[bot] left review comments

@vinnyhoward vinnyhoward vinnyhoward approved these changes

Assignees

@brianacnguyen brianacnguyen

Labels

No QA Needed Apply this label when your PR does not need any QA effort. release-7.54.0 Issue or pull request that will be included in release 7.54.0 size-XL skip-sonar-cloud Only used for bypassing sonar cloud when failures are not relevant to the changes. team-design-system All issues relating to design system in Mobile

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Bug]: Button not firing onPress consistently on Android

5 participants

@brianacnguyen @codecov-commenter @ieow @vinnyhoward @metamaskbot