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

check haptic system settings before triggering #28

Merged
merged 7 commits into from
Apr 8, 2019
Merged

check haptic system settings before triggering #28

merged 7 commits into from
Apr 8, 2019

Conversation

adkenyon
Copy link
Contributor

Haptic feedback triggers despite the user's system settings. (If they turn off haptic via the accessibility settings then haptic does disable).

The change checks the haptic system settings prior to triggering and will return if haptic is disabled.

@mkuczera
Copy link
Owner

mkuczera commented Apr 1, 2019

Hi, thank you for the contribution.
Due to the fact that i actually don´t know if some people want this behaviour, i would prefer to have it as an optional settings value for the trigger like "ignoreAndroidSettings"

Greetings

Michael

@adkenyon
Copy link
Contributor Author

adkenyon commented Apr 1, 2019

Hello @mkuczera,

What are your thoughts on the trigger function arguments. I can either add a third parameter or change the second parameter to an options object (examples below). I guess a third option would be to make the second argument dynamic, in that it accepts a bool or config object. Adding a thrid argument is the easiest to implement, but long term having a config object is much more extensible.

// Using a 3rd parameter
ReactNativeHapticFeedback.trigger('impactLight', true, true);



// using a config object
const options = {
    enableVibrateFallback: true,
    ignoreAndroidSystemSettings: true
};

ReactNativeHapticFeedback.trigger('impactLight', options);

@adkenyon
Copy link
Contributor Author

adkenyon commented Apr 4, 2019

Hey @mkuczera,

After sleeping on it I decided to go with the third option. You can use the same trigger args as seen in v1.6 (type:string, enableVibrateFallback:bool) and the updated args (type:string, options:object). I've updated the type definition and cos to reflect the new arguments. Let me know what you think

@mkuczera
Copy link
Owner

mkuczera commented Apr 5, 2019

Hey, thanks that is looking awesome so far. Wanted to go for the options argument as well. I will check everything and create a new release on the upcoming weekend :)

Greetings
Michael

@mkuczera mkuczera merged commit 50d7025 into mkuczera:master Apr 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants