Skip to content

Guard ActionSheet hide() to skip side effects when not visible - #7508

Closed
Rohit3523 wants to merge 1 commit into
developfrom
fix/action-sheet-hide-guard
Closed

Guard ActionSheet hide() to skip side effects when not visible#7508
Rohit3523 wants to merge 1 commit into
developfrom
fix/action-sheet-hide-guard

Conversation

@Rohit3523

@Rohit3523 Rohit3523 commented Jul 23, 2026

Copy link
Copy Markdown
Member

Proposed changes

Guard ActionSheet.hide() to skip all side effects (haptic, keyboard dismiss, toast) when no sheet is visible. On cold start, hideActionSheetRef() was called 3 times via redundant VoIP reset paths — disconnect(), checkVoipPermission, and the PERMISSIONS.SET re-check — each firing Haptics.impactAsync despite no sheet being open.

Issue(s)

https://rocketchat.atlassian.net/browse/SUP-1079

How to test or reproduce

  1. Cold open the app
  2. Observe that no unexpected vibration occurs during the connecting/updating/connected header transitions
  3. Long-press a room item to open the ActionSheet, then dismiss it — haptic should still fire

Screenshots

N/A

Types of changes

  • Bugfix (non-breaking change which fixes an issue)
  • Improvement (non-breaking change which improves a current function)
  • New feature (non-breaking change which adds functionality)
  • Documentation update (if none of the other choices apply)

Checklist

  • I have read the CONTRIBUTING doc
  • I have signed the CLA
  • Lint and unit tests pass locally with my changes
  • I have added tests that prove my fix is effective or that my feature works (if applicable)
  • I have added necessary documentation (if applicable)
  • Any dependent changes have been merged and published in downstream modules

Summary by CodeRabbit

  • Bug Fixes
    • Prevented redundant dismissal actions when the action sheet is already hidden.
    • Improved reliability by avoiding unnecessary side effects during hidden-state interactions.

hide() was called on cold start via 3 redundant VoIP reset paths
(disconnect, checkVoipPermission, PERMISSIONS.SET re-check),
each firing Haptics.impactAsync and other side effects despite
no sheet being open. Adding !isVisible guard makes these no-ops.
@Rohit3523
Rohit3523 had a problem deploying to approve_e2e_testing July 23, 2026 17:22 — with GitHub Actions Failure
@coderabbitai

coderabbitai Bot commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 4d87ddff-e5bf-4d1b-8c00-130c231886b0

📥 Commits

Reviewing files that changed from the base of the PR and between 1d9b5cc and 9fd31e9.

📒 Files selected for processing (1)
  • app/containers/ActionSheet/ActionSheet.tsx

Walkthrough

The ActionSheet hide handler now exits early when the sheet is already hidden, preventing redundant dismissal side effects.

Changes

ActionSheet visibility handling

Layer / File(s) Summary
Guard hidden ActionSheet dismissal
app/containers/ActionSheet/ActionSheet.tsx
The hide function checks isVisible and skips dismissal, keyboard dismissal, and light haptic feedback when the sheet is not visible.

Estimated code review effort: 1 (Trivial) | ~2 minutes

Suggested labels: type: bug

Suggested reviewers: diegolmello, otaviostasiak


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant