Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow Modal to handle hardware escape key in the same way the back button is handled #31564

Closed

Conversation

levibuzolic
Copy link
Contributor

Summary

On Android, when a hardware keyboard is connected pressing the escape key will partially dismiss an active <Modal> without calling the onRequestClose callback. The modal will disappear, but I beleive the underlying activity may still be present, blocking interaction with the main app below and leaving things in a partially broken state.

This code change allows the escape key to be handled in the same way as the hardware back button, calling the onRequestClose and allowing the developer to decide the behaviour.

This issue isn't present on iOS, so no change is required there.

Changelog

[Android] [Fixed] - Fix Modal being dismissed incorrectly when pressing escape on a hardware keyboard

Test Plan

I've tested this manually locally, but unsure if it's possible to test in an automated way as the emulator didn't respond to a hardware escape key in the same way as a physical device.

@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 May 21, 2021
@analysis-bot
Copy link

Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 9,274,328 +53
android hermes armeabi-v7a 8,790,882 +50
android hermes x86 9,735,815 +45
android hermes x86_64 9,701,890 +49
android jsc arm64-v8a 10,881,757 -61
android jsc armeabi-v7a 10,391,356 -65
android jsc x86 10,910,002 -57
android jsc x86_64 11,517,964 -71

Base commit: 59abb5f

@analysis-bot
Copy link

Platform Engine Arch Size (bytes) Diff
ios - universal n/a --

Base commit: ffab8e3

@PCHGrant
Copy link

Hello

@lunaleaps lunaleaps self-assigned this Jun 8, 2021
@lunaleaps
Copy link
Contributor

Ohoo this makes sense. Let me import and see if we can add an E2E test case for this.

@facebook-github-bot
Copy link
Contributor

@lunaleaps has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@lunaleaps
Copy link
Contributor

@levibuzolic When you say hardware escape key, did you hook up a keyboard to your Android device and press the escape key? Is that the flow you're referring to?

@levibuzolic
Copy link
Contributor Author

@lunaleaps yes, that's correct.

@github-actions github-actions bot added Needs: Attention Issues where the author has responded to feedback. and removed Needs: Author Feedback labels Aug 3, 2021
@facebook-github-bot
Copy link
Contributor

@lunaleaps merged this pull request in f51773e.

@facebook-github-bot facebook-github-bot added the Merged This PR has been merged. label Aug 14, 2021
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. Merged This PR has been merged. Needs: Attention Issues where the author has responded to feedback.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants