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

[TIMOB-16080] Removes (Annoying) touch sound that are on EVERY view. #5373

Closed
wants to merge 1 commit into from
Closed

[TIMOB-16080] Removes (Annoying) touch sound that are on EVERY view. #5373

wants to merge 1 commit into from

Conversation

gstreetmedia
Copy link
Contributor

Touch sounds on everything lead to a very confusing user experience.
This changes behavior such that all views will have no touch sound (by default), but those views which are specifically set as touchEnabled = true will have the touchsound.

Only views that are touchable (click event) should have a touch sound.
Touch sounds on everything lead to a very confusing user experience.
@Sophrinix
Copy link
Contributor

+1

Please merge this once the conflicts have been resolved. Are there side effects or some downside to this that isn't obvious?

@ashcoding
Copy link
Contributor

@gstreetmedia Are you able to resolve the merge conflicts?

Jira for ref: https://jira.appcelerator.org/browse/TIMOB-16080

@hansemannn
Copy link
Collaborator

Closing this PR for now, since the merge conflicts have not been resolved over the last 6 months and the CLA is not signed. Feel free to open a new PR to the master and we can review it. Thanks!

@hansemannn hansemannn closed this Mar 23, 2016
@gstreetmedia
Copy link
Contributor Author

I'm pretty sure I signed the CLA jeff sent me two years ago.

@hansemannn
Copy link
Collaborator

We migrated that process a while ago to incorporate with Github. Please also make sure to resolve the merge conflicts. Will reopen, you can sign it http://cla.appcelerator.com. Thanks!

@hansemannn hansemannn reopened this Mar 23, 2016
//If touchEnabled is specifically set, enable or disable touch sound
touchable.setSoundEffectsEnabled(clickable);
} else {
touchable.setSoundEffectsEnabled(false);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By making this change, it would change the default (native) behavior. I'm good with deactivating the sound when touches are not enabled, but it shouldn't change the default behavior. What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure. The default behavior of TI is that touch sounds are enabled on every kind of view, no matter whether or not that view has any kind of touch / click event applied to it. This not the default behavior in native-built applications. If you don't do this is some kind of global fashion, then you would have to apply this setting to every nested element of a view. Say a view in comprised of three additional views, all without interactivity, then you would specifically have to apply touchEnabled = false to each and every view. To make matters worse, on iOS setting touchEnabled = false on a parent view, would disable interactive events on all child views (not true in Android). As such, this was the only feasible place I could think / find to implement this logic.

The only way a developer has to make this choice currently is styles.xml, which is an all or nothing proposition.

<style name="AppTheme" parent="AppBaseTheme"> false </style>

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ashcoding, your thoughts?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ping @ashcoding

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i'd be fine to disable it for 6.0 (accept the suggested PR) and just make a ti flag in tiapp.xml which allows you to revert to previous behavior.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alright. So let's introduce the property <property name="ti.android.soundEffectsEnabled" type="bool" /> in the naming convention like ti.android.bug2373.finishfalseroot and merge this PR. @gstreetmedia can you apply this late change? If not, we would get an Android engineer of our team on it and merge this before. Thanks in advance!

@hansemannn
Copy link
Collaborator

Closing in favor of #8816 🚀

@hansemannn hansemannn closed this Feb 3, 2017
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.

None yet

6 participants