-
Notifications
You must be signed in to change notification settings - Fork 24.4k
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
Adds Android click sound to Touchables #17183
Conversation
@facebook-github-bot label Needs more information Generated by 🚫 dangerJS |
_playTouchSound: function() { | ||
UIManager.playTouchSound(); | ||
}, | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no-trailing-spaces: Trailing spaces not allowed.
this.touchableHandlePress(e); | ||
} | ||
} | ||
|
||
this.touchableDelayTimeout && clearTimeout(this.touchableDelayTimeout); | ||
this.touchableDelayTimeout = null; | ||
}, | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no-trailing-spaces: Trailing spaces not allowed.
@@ -743,14 +743,21 @@ var TouchableMixin = { | |||
this._startHighlight(e); | |||
this._endHighlight(e); | |||
} | |||
if(Platform.OS === 'android') { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
keyword-spacing: Expected space(s) after "if".
Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need the corporate CLA signed. If you have received this in error or have any questions, please contact us at [email protected]. Thanks! |
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks! |
Update to latest version
@facebook-github-bot label Android Generated by 🚫 dangerJS |
@akriger I tried to find reviewers for this pull request and wanted to ping them to take another look. However, based on the blame information for the files in this pull request I couldn't find any reviewers. This sometimes happens when the files in the pull request are new or don't exist on master anymore. Is this pull request still relevant? If yes could you please rebase? In case you know who has context on this code feel free to mention them in a comment (one person is fine). Thanks for reading and hope you will continue contributing to the project. |
Update to latest React Native Version
@hramos @janicduplessis is there anything else I need to do for this PR? I see one of the checks failed but, not sure if that has anything to do with my PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@hramos is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
Summary: Android apps play a touch sound on press, as long as you have "Touch sounds" enabled in the settings. As and Android user, when building my app using React Native, one of the first things I noticed was that there were not any touch sounds. This is missing from React Native and there have been multiple PRs to have this implemented, but no success. This PR iterates over [facebook#6825](facebook#6825) and [facebook#11136](facebook#11136) This PR keeps it simple by only implementing the enhancement for Android, as iOS apps typically do not use touch sounds, and follows the users' system settings for whether or not the sound is played. I have manually tested this on multiple devices and emulators with zero problems [ANDROID] [ENHANCEMENT] [UIManagerModule.java]- Adds Android click sound to touchables [ANDROID] [ENHANCEMENT] [Touchable] - Adds Android click sound to touchables Closes facebook#17183 Differential Revision: D7560327 Pulled By: hramos fbshipit-source-id: ce1094c437541bc677c7d64b0dba343dd9574422
Summary: Android apps play a touch sound on press, as long as you have "Touch sounds" enabled in the settings. As and Android user, when building my app using React Native, one of the first things I noticed was that there were not any touch sounds. This is missing from React Native and there have been multiple PRs to have this implemented, but no success. This PR iterates over [facebook#6825](facebook#6825) and [facebook#11136](facebook#11136) This PR keeps it simple by only implementing the enhancement for Android, as iOS apps typically do not use touch sounds, and follows the users' system settings for whether or not the sound is played. I have manually tested this on multiple devices and emulators with zero problems [ANDROID] [ENHANCEMENT] [UIManagerModule.java]- Adds Android click sound to touchables [ANDROID] [ENHANCEMENT] [Touchable] - Adds Android click sound to touchables Closes facebook#17183 Differential Revision: D7560327 Pulled By: hramos fbshipit-source-id: ce1094c437541bc677c7d64b0dba343dd9574422
Summary: Android apps play a touch sound on press, as long as you have "Touch sounds" enabled in the settings. As and Android user, when building my app using React Native, one of the first things I noticed was that there were not any touch sounds. This is missing from React Native and there have been multiple PRs to have this implemented, but no success. This PR iterates over [facebook#6825](facebook#6825) and [facebook#11136](facebook#11136) This PR keeps it simple by only implementing the enhancement for Android, as iOS apps typically do not use touch sounds, and follows the users' system settings for whether or not the sound is played. I have manually tested this on multiple devices and emulators with zero problems [ANDROID] [ENHANCEMENT] [UIManagerModule.java]- Adds Android click sound to touchables [ANDROID] [ENHANCEMENT] [Touchable] - Adds Android click sound to touchables Closes facebook#17183 Differential Revision: D7560327 Pulled By: hramos fbshipit-source-id: ce1094c437541bc677c7d64b0dba343dd9574422
Summary: Android apps play a touch sound on press, as long as you have "Touch sounds" enabled in the settings. As and Android user, when building my app using React Native, one of the first things I noticed was that there were not any touch sounds. This is missing from React Native and there have been multiple PRs to have this implemented, but no success. This PR iterates over [facebook#6825](facebook#6825) and [facebook#11136](facebook#11136) This PR keeps it simple by only implementing the enhancement for Android, as iOS apps typically do not use touch sounds, and follows the users' system settings for whether or not the sound is played. I have manually tested this on multiple devices and emulators with zero problems [ANDROID] [ENHANCEMENT] [UIManagerModule.java]- Adds Android click sound to touchables [ANDROID] [ENHANCEMENT] [Touchable] - Adds Android click sound to touchables Closes facebook#17183 Differential Revision: D7560327 Pulled By: hramos fbshipit-source-id: ce1094c437541bc677c7d64b0dba343dd9574422
Summary: Android apps play a touch sound on press, as long as you have "Touch sounds" enabled in the settings. As and Android user, when building my app using React Native, one of the first things I noticed was that there were not any touch sounds. This is missing from React Native and there have been multiple PRs to have this implemented, but no success. This PR iterates over [facebook#6825](facebook#6825) and [facebook#11136](facebook#11136) This PR keeps it simple by only implementing the enhancement for Android, as iOS apps typically do not use touch sounds, and follows the users' system settings for whether or not the sound is played. I have manually tested this on multiple devices and emulators with zero problems [ANDROID] [ENHANCEMENT] [UIManagerModule.java]- Adds Android click sound to touchables [ANDROID] [ENHANCEMENT] [Touchable] - Adds Android click sound to touchables Closes facebook#17183 Differential Revision: D7560327 Pulled By: hramos fbshipit-source-id: ce1094c437541bc677c7d64b0dba343dd9574422
Summary: Currently, every time a touchable is pressed on Android, a system sound is played. It was added in the PR #17183. There is no way to disable it, except disabling touch on sound on the system level. I am pretty sure there are cases when touches should be silent and there should be an option to disable it. Related PRs - #17183, #11136 [Android][added] - Added a touchSoundDisabled prop to Touchable. If true, doesn't system sound on touch. Pull Request resolved: #24666 Differential Revision: D15166582 Pulled By: cpojer fbshipit-source-id: 48bfe88f03f791e3b9c7cbd0e2eed80a2cfba8ee
Motivation
Android apps play a touch sound on press, as long as you have "Touch sounds" enabled in the settings. As and Android user, when building my app using React Native, one of the first things I noticed was that there were not any touch sounds. This is missing from React Native and there have been multiple PRs to have this implemented, but no success.
Related PRs
This PR iterates over #6825 and #11136
This PR keeps it simple by only implementing the enhancement for Android, as iOS apps typically do not use touch sounds, and follows the users' system settings for whether or not the sound is played.
Test Plan
I have manually tested this on multiple devices and emulators with zero problems
Release Notes
[ANDROID] [ENHANCEMENT] [UIManagerModule.java]- Adds Android click sound to touchables
[ANDROID] [ENHANCEMENT] [Touchable] - Adds Android click sound to touchables