Skip to content

fix: guard ActionSheet hide() to skip side effects when not visible - #7509

Merged
Rohit3523 merged 2 commits into
developfrom
fix/action-sheet-hide-guard
Jul 28, 2026
Merged

fix: guard ActionSheet hide() to skip side effects when not visible#7509
Rohit3523 merged 2 commits 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) 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.
    • Avoided unnecessary keyboard and haptic feedback during repeated dismiss attempts.

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:26 — with GitHub Actions Failure
@coderabbitai

coderabbitai Bot commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Walkthrough

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

Changes

ActionSheet visibility handling

Layer / File(s) Summary
Guard redundant dismissal
app/containers/ActionSheet/ActionSheet.tsx
hide returns early when isVisible is false before triggering dismissal side effects.

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

Possibly related PRs

Suggested labels: type: bug

Suggested reviewers: diegolmello, otaviostasiak

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: guarding ActionSheet hide() to avoid side effects when it is already not visible.

Warning

Review ran into problems

🔥 Problems

Errors were encountered while retrieving linked issues.

Errors (1)
  • SUP-1079: Request failed with status code 401

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.

@github-actions

Copy link
Copy Markdown

@github-actions

Copy link
Copy Markdown

iOS Build Available

Rocket.Chat 4.75.0.109398

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

LGTM

@Rohit3523
Rohit3523 had a problem deploying to approve_e2e_testing July 28, 2026 12:16 — with GitHub Actions Failure
@Rohit3523
Rohit3523 merged commit 755b309 into develop Jul 28, 2026
6 of 9 checks passed
@Rohit3523
Rohit3523 deleted the fix/action-sheet-hide-guard branch July 28, 2026 12:23
diegolmello added a commit that referenced this pull request Jul 31, 2026
The base branch was rebased, so its 13 commits (the five reverts plus
follow-ups) arrived with new SHAs while dry-badger already carried them
as identical patches. Every conflict is that duplicate content, resolved
to dry-badger's side, which keeps the socket health work layered on top.

Only #7509 (ActionSheet hide guard) and #7510 (UserPreferences.getBool)
are new here. jest.config.js keeps the @rocket.chat/sdk and tiny-events
transform entries that ddpSocket.test.ts needs; the rebased base dropped
them along with the test file it deleted.
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.

2 participants