Skip to content

fix(android): intercept ESCAPE key ACTION_DOWN in Modal to prevent premature dismiss#56498

Open
taibin wants to merge 1 commit intofacebook:mainfrom
taibin:fix/modal-escape-key-handling
Open

fix(android): intercept ESCAPE key ACTION_DOWN in Modal to prevent premature dismiss#56498
taibin wants to merge 1 commit intofacebook:mainfrom
taibin:fix/modal-escape-key-handling

Conversation

@taibin
Copy link
Copy Markdown

@taibin taibin commented Apr 20, 2026

Summary

Fix Android Modal's handling of the ESCAPE key so that JavaScript is reliably notified (via onRequestClose) instead of the dialog dismissing itself before JS sees the event.

Problem

ReactModalHostView installs an OnKeyListener that only reacts on ACTION_UP for KEYCODE_BACK / KEYCODE_ESCAPE, calling handleCloseAction() to let JS decide whether to close.

However, Android's Dialog.onKeyDown() handles these two keys asymmetrically:

  • KEYCODE_BACKACTION_DOWN only calls event.startTracking(); the dialog is dismissed on ACTION_UP via onBackPressed(). Our ACTION_UP interception works fine.
  • KEYCODE_ESCAPEACTION_DOWN directly calls Dialog.cancel(). By the time our ACTION_UP handler would run, the dialog is already being dismissed, so onRequestClose is never delivered to JS.

Net effect: pressing ESCAPE on Android closes the Modal unconditionally, bypassing JS (breaks apps that rely on onRequestClose for confirmation dialogs, unsaved-changes prompts, etc.).

Fix

Restructure the key listener to branch on keyCode first:

  • For KEYCODE_BACK / KEYCODE_ESCAPE:
    • Consume ACTION_DOWN (return true) to prevent Dialog.onKeyDown() from dismissing the dialog.
    • On ACTION_UP, call handleCloseAction() as before, giving JS the final say.
  • For all other keys, preserve the previous behavior of forwarding ACTION_UP to the current Activity (needed by the dev menu, etc.).

Test Plan

  • Render a <Modal visible> on Android, attach onRequestClose handler, press the hardware BACK button — handler fires, modal stays open unless JS sets visible={false}. (Regression coverage: unchanged behavior.)
  • Same setup, press ESCAPE on a connected keyboard / emulator — handler now fires correctly; previously the modal closed without JS notification.
  • Open the dev menu shortcut while a modal is not showing — still works (non-BACK/ESCAPE keys still forwarded to Activity).

Changelog

[ANDROID] [FIXED] - Modal's onRequestClose is now correctly invoked when pressing ESCAPE on Android (previously the dialog was dismissed before JS was notified).

…emature dismiss

The Modal's OnKeyListener previously only handled BACK/ESCAPE on ACTION_UP,
but Dialog.onKeyDown() directly calls cancel() for KEYCODE_ESCAPE on
ACTION_DOWN (unlike KEYCODE_BACK, which defers to onBackPressed on ACTION_UP).
As a result, ESCAPE closed the dialog before JS was ever notified.

Restructure the listener to branch on keyCode first:
- For BACK/ESCAPE: consume ACTION_DOWN to block Dialog's default handling,
  then call handleCloseAction() on ACTION_UP so JS can decide whether to close.
- For all other keys: keep the existing behavior of forwarding ACTION_UP to
  the current Activity (needed by the dev menu, etc.).
@meta-cla meta-cla 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 Apr 20, 2026
@github-actions
Copy link
Copy Markdown

Caution

Missing Changelog

Please add a Changelog to your PR description. See Changelog format

@facebook-github-tools facebook-github-tools Bot added the Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. label Apr 20, 2026
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.

1 participant