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

Haptics #5

Merged
merged 40 commits into from Aug 24, 2020
Merged

Haptics #5

merged 40 commits into from Aug 24, 2020

Conversation

jcesarmobile
Copy link
Member

@jcesarmobile jcesarmobile commented Jul 30, 2020

haptics/src/web.ts Outdated Show resolved Hide resolved
haptics/src/web.ts Outdated Show resolved Hide resolved
haptics/src/web.ts Outdated Show resolved Hide resolved
haptics/src/web.ts Outdated Show resolved Hide resolved
haptics/src/web.ts Outdated Show resolved Hide resolved
haptics/src/web.ts Outdated Show resolved Hide resolved
haptics/src/web.ts Outdated Show resolved Hide resolved
haptics/src/web.ts Outdated Show resolved Hide resolved
@imhoffd imhoffd linked an issue Jul 31, 2020 that may be closed by this pull request
@cameronrr
Copy link

Thanks for all the work on this. Two points for discussion below:

Message for'API Not Available'
In the Share plugin, a false feature detection returns 'Web Share API not available'. Is it worth having a consistent messaging from Capacitor plugins in this scenario i.e. if Haptics is not available, to output 'Web Haptics API not available' instead of 'Vibrate API not available in this browser'. Specificity could follow the standard message if needed i.e. 'Web Haptics API not available - not supported by this browser'. There might be value in a consistent format not only for clearer plugin development but for developers to quickly recognise the pattern and that it is from the Capacitor web plugin.

Vibrate Options
The Web API supports not only a number as the duration parameter, but also an array (Number[]) to define a vibration pattern. Is it possible to handle this in the web plugin to leverage the full set of Vibrate API features? I think the main consideration is the interaction with native plugins if the options/capabilities are different. I think the goal is a single interface for developers regardless of device so not sure how this would be approached.

@imhoffd
Copy link
Contributor

imhoffd commented Aug 3, 2020

@cameronrr Good input, thanks! I agree, we need to be consistent about error messages. I hope to have a code associated with each error so you can just check if (e.code === 'ERR_WEB_API_UNAVAILABLE") for example. As for the vibrate options, I'm not sure it's in the scope of this plugin. We want people to use the high-level haptics methods. We've debated removing the vibrate() method completely. I think there's room for an "advanced vibration" plugin in the Capacitor community (and keep in mind, iOS doesn't support patterns or even different length vibrations).

@jcesarmobile jcesarmobile marked this pull request as ready for review August 18, 2020 14:57
jcesarmobile and others added 5 commits August 19, 2020 12:09
Co-authored-by: Dan Imhoff <dwieeb@gmail.com>
Co-authored-by: Dan Imhoff <dwieeb@gmail.com>
Co-authored-by: Dan Imhoff <dwieeb@gmail.com>
Co-authored-by: Dan Imhoff <dwieeb@gmail.com>
haptics/ios/Plugin/Haptics.swift Outdated Show resolved Hide resolved
haptics/ios/Plugin/Haptics.swift Outdated Show resolved Hide resolved
@imhoffd imhoffd added this to In progress 🤺 in Capacitor Engineering ⚡️ via automation Aug 21, 2020
@imhoffd imhoffd merged commit 95322d3 into main Aug 24, 2020
Capacitor Engineering ⚡️ automation moved this from In progress 🤺 to Done 🎉 Aug 24, 2020
@imhoffd imhoffd deleted the haptics branch August 24, 2020 17:15
@nicolocarpignoli
Copy link

Hi everybody.
It seems that from iOS 13+ it is possible to use CoreHaptics API on iOS and handle more fine grained haptics feedbacks.
For example, we can manage the duration https://developer.apple.com/documentation/corehaptics/chhapticevent/3081790-duration?language=objc.

Does this PR introduced such behaviour, or it is not based on CoreHaptics?

Thank you

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

Haptics Web Implementation
6 participants