-
Notifications
You must be signed in to change notification settings - Fork 8
Added getVolume() #2
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
base: master
Are you sure you want to change the base?
Conversation
Added the option to get the current volume
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.
Thank you for your submission Steve! I went through your PR and made some comments. Please review my comments and make changes. After everything checks out I'll make merge the PR.
@@ -1,4 +1,4 @@ | |||
# cordova-volume-control | |||
# cordova-simple-volume |
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.
Is see you changed the name of the plugin. I'm assuming this is because another plugin has claimed the npm namespace. I'm open to changing this, but it would be good if releases of this package would be in sync with this repo.
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.
I didn't change the name I just folked a folk which had already changed the name? I have no preference on name
## Installation | ||
|
||
cordova plugin add https://github.com/boedy/cordova-volume-control --save | ||
cordova plugin add https://github.com/online-data-intelligence/cordova-simple-volume |
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.
Pointing to wrong repo
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.
This was just for my testing on my folk. I will put everything to point to the main repo
}, | ||
"repository": { | ||
"type": "git", | ||
"url": "git+https://github.com/online-data-intelligence/cordova-simple-volume.git" |
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.
Wrong repo
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.
This was just for my testing on my folk. I will put everything to point to the main repo
"author": "Boudewijn Geiger", | ||
"license": "Apache 2.0", | ||
"bugs": { | ||
"url": "https://github.com/online-data-intelligence/cordova-simple-volume/issues" |
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.
wrong repo
setVolume: function (volume) { | ||
cordova.exec(null, null, "VolumeControl", "setVolumeCommand", [volume]); | ||
}, | ||
getVolume: function () { |
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.
iOS does not support es6 syntax. So arrow functions (=>) don't work.
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.
Thats strange as the plugin works on my iPhone? I will remove and test without the arrow functions.
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.
Ah it seems it works on the newer iOS safari versions . It would be good to support older versions too though (e.g iPod touch 5th generation is on iOS 9).
Added getVolume() which returns a promise with the current volume set.