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

feat(HyperTrack): add plugin #1816

Merged
merged 45 commits into from
Jul 18, 2017
Merged

feat(HyperTrack): add plugin #1816

merged 45 commits into from
Jul 18, 2017

Conversation

danielolivaresd
Copy link
Contributor

@danielolivaresd danielolivaresd commented Jul 17, 2017

Plugin that helps with location based services (i.e. track users, assign tasks, etc)

* @see {@link https://github.com/hypertrack/hypertrack-cordova/blob/master/www/HyperTrack.js#33|HyperTrack.js createAndAssignAction implementation}
* @see {@link https://docs.hypertrack.com/api/entities/action.html#the-action-object|HyperTrack API Documentation for Action}
*
* @usage
Copy link
Collaborator

Choose a reason for hiding this comment

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

All usage examples should stay at the top, in the @Plugin() decorator's doc section.

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 see. I checked other plugins, (e.g. EstimoteBeacons) and followed that convention, but that's not a problem. Should be done in some minutes.

@danielolivaresd
Copy link
Contributor Author

Let me know if anything else is needed before the PR is merged

/**
* Create a new user to identify the current device or get a user from a lookup id.
*
* @param {String} [name] User's name
Copy link
Collaborator

Choose a reason for hiding this comment

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

The params are documented as optional, but the method signature defines them as mandatory. Please remove the brackets.

plugin: 'cordova-plugin-hypertrack',
pluginRef: 'cordova.plugins.HyperTrack',
repo: 'https://github.com/hypertrack/hypertrack-cordova',
platforms: ['android']
Copy link
Collaborator

Choose a reason for hiding this comment

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

Capitalize the word android.

* Returns given text. For testing purposes.
*
* @param {String} [text] Given text to print
*
Copy link
Collaborator

Choose a reason for hiding this comment

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

Remove empty lines

*
* @description HyperTrack cordova plugin wrapper for Ionic Native. Location-based services provider. iOS is being implemented.
*
* @see {@link https://docs.hypertrack.com/sdks/cordova/reference.html#methods|Hypertrack Cordova Methods Reference}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Refrain from using @see and @link tags. They are not supported here. You can use Markdown to link to external resources if you need to.

Copy link
Contributor Author

@danielolivaresd danielolivaresd Jul 18, 2017

Choose a reason for hiding this comment

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

Should I stop using them throughout the whole file or only at the Plugin decorator?

* @see {@link https://docs.hypertrack.com/sdks/android/setup.html#step-3-enable-location-permissions|HyperTrack Android SDK Setup}
*/
@Cordova({
platforms: ['android']
Copy link
Collaborator

Choose a reason for hiding this comment

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

Since the plugin only supports Android platform, there is no need to define this here as well.

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 have been in touch with the maintainer of the HyperTrack cordova plugin, and iOS support is coming soon, however the methods that I specified that they only work on Android will most probably only be available for Android, even when iOS support comes. This is: checkLocationPermission, requestPermissions, checkLocationServices, and requestLocationServices will only work in Android even when iOS is supported.

This is the reason why I specified it, but I can erase it, and once iOS is supported it can be put back.

Copy link
Collaborator

Choose a reason for hiding this comment

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

The config passed to @Plugin() indicates that Android is the only supported platform.

@ihadeed
Copy link
Collaborator

ihadeed commented Jul 18, 2017

@K1N5L4Y3R just submitted a new review. Once you fix those issues, I will review it once more to make sure that the signatures match the original plugin.

@ihadeed
Copy link
Collaborator

ihadeed commented Jul 18, 2017

Looks good now.

@ihadeed ihadeed merged commit 47fa44c into danielsogl:master Jul 18, 2017
@danielolivaresd
Copy link
Contributor Author

Great, thanks!

Just wondering, what does the beta tag do? and approx. when can I expect to see this plugin in a release?

@ihadeed
Copy link
Collaborator

ihadeed commented Jul 18, 2017

Should be released over the next few days (maybe today).

The beta tags marks it as a beta plugin I'm the docs. This is because it's a new plugin and it's hasn't proven relibility yet.

BuddyLReno pushed a commit to BuddyLReno/ionic-native that referenced this pull request Aug 28, 2017
* feat(HyperTrack): add base implementation for HyperTrack

* feat(HyperTrack): fix getOrCreateUser()

* docs(HyperTrack): add note about publishable key

* feat(HyperTrack): add requestPermissions

* feat(HyperTrack): add startTracking

* docs(HyperTrack): add usage for calls

* feat(HyperTrack): add stopTracking

* docs(HyperTrack): add stopTracking note

* feat(HyperTrack): add setUserId

* feat(HyperTrack): add getCurrentLocation

* style(HyperTrack): reorder functions

* feat(HyperTrack): add createAndAssignAction

* feat(HyperTrack): add completeAction

* feat(HyperTrack): add completeActionWithLookupId

* feat(HyperTrack): add checkLocationPermission

* docs(HyperTrack): add usage for requestPermissions

* docs(HyperTrack): add android note

* feat(HyperTrack): add checkLocationServices

* feat(HyperTrack): add requestLocationServices

* feat(HyperTrack): add base implementation for HyperTrack

* feat(HyperTrack): fix getOrCreateUser()

* docs(HyperTrack): add note about publishable key

* feat(HyperTrack): add requestPermissions

* feat(HyperTrack): add startTracking

* docs(HyperTrack): add usage for calls

* feat(HyperTrack): add stopTracking

* docs(HyperTrack): add stopTracking note

* feat(HyperTrack): add setUserId

* feat(HyperTrack): add getCurrentLocation

* style(HyperTrack): reorder functions

* feat(HyperTrack): add createAndAssignAction

* feat(HyperTrack): add completeAction

* feat(HyperTrack): add completeActionWithLookupId

* feat(HyperTrack): add checkLocationPermission

* docs(HyperTrack): add usage for requestPermissions

* docs(HyperTrack): add android note

* feat(HyperTrack): add checkLocationServices

* feat(HyperTrack): add requestLocationServices

* docs(HyperTrack): Fix plugin usage

* fix(HyperTrack): helloWorld param and docs

* docs(HyperTrack): document all functions

* docs(HyperTrack): move all usage documentation up

* docs(HyperTrack): fix documentation

* Add beta tag
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

2 participants