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-5436] iOS: Expose System Sound Services #8396

Merged
merged 5 commits into from Oct 31, 2016

Conversation

kopiro
Copy link
Contributor

@kopiro kopiro commented Sep 19, 2016

JIRA: https://jira.appcelerator.org/browse/TIMOB-5436

You can use this module to provide audible alerts.

This module differs from the Sound module because it honors the ringtone volume, not the Music volume.

Know more about System Sound Services.

Example:

Ti.Media.createAlert({url: '/sounds/test.wav' }).play();

@kopiro kopiro changed the title Expose iOS System Sound Services [TIMOB-5436] Expose iOS System Sound Services Sep 19, 2016
Copy link
Collaborator

@hansemannn hansemannn left a comment

Choose a reason for hiding this comment

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

First CR-cycle done, please address the review-comments.

description: |
You can use this module to provide audible alerts.

You can use it to play short (30 seconds or shorter) sounds. The interface does not provide level, positioning, looping, or timing control, and does not support simultaneous playback: You can play only one sound at a time.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Watch out for proper line-breaks (see other API's), so the line does not get too long.

Use the <Titanium.Media.createAlert> method to create a `Alert` object.

Know more about [System Sound Services](https://developer.apple.com/reference/audiotoolbox/1657326-system_sound_services).

Copy link
Collaborator

Choose a reason for hiding this comment

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

Remove the empty lines here, it's formatted as a paragraph by the docs-generator.

Know more about [System Sound Services](https://developer.apple.com/reference/audiotoolbox/1657326-system_sound_services).

extends: Titanium.Proxy
since: "5.5.0"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Change to 6.1.0

summary: Starting playing the alert.

events:

Copy link
Collaborator

Choose a reason for hiding this comment

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

No events? If not, remove it.

platforms: [iphone, ipad]
methods:
- name: play
summary: Starting playing the alert.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Language: Start playing the alert.

@@ -0,0 +1,24 @@
/**
* Appcelerator Titanium Mobile
* Copyright (c) 2009-2010 by Appcelerator, Inc. All Rights Reserved.
Copy link
Collaborator

Choose a reason for hiding this comment

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

2009-2016

example: |
Simple example of playing a WAVE file from the Resources directory.

var player = Ti.Media.createAlert({url:"Alert.wav"});
Copy link
Collaborator

Choose a reason for hiding this comment

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

Use alert.wav to be more handy in naming-conventions.

SystemSoundID sound;
}

@property (nonatomic,readonly) NSURL *url;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we need this as a property as well? Better make a usual getter -(NSURL*)url where you create the url instead of just returning it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not understanding this.

}
} else if ([url_ isKindOfClass:[TiBlob class]]) {
TiBlob *blob = (TiBlob*)url_;
if ([blob type]==TiBlobTypeFile){
Copy link
Collaborator

Choose a reason for hiding this comment

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

Format: if ([blob type] == TiBlobTypeFile) {


-(void)play:(id)args
{
if (url == nil) return;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Use -(void)play:(id)unused

@hansemannn hansemannn changed the title [TIMOB-5436] Expose iOS System Sound Services [TIMOB-5436] iOS: Expose System Sound Services Sep 19, 2016
@sgtcoolguy
Copy link
Contributor

ping @kopiro @hansemannn What is left over to get this merged?

@sgtcoolguy sgtcoolguy added this to the 6.1.0 milestone Oct 31, 2016
@kopiro
Copy link
Contributor Author

kopiro commented Oct 31, 2016

Nothing for me, tested and worked after @hansemannn PR

@hansemannn
Copy link
Collaborator

CR + FT passed, PR approved!

@hansemannn hansemannn merged commit e5d2679 into tidev:master Oct 31, 2016
@hansemannn
Copy link
Collaborator

We may also want unit-tests to ensure the integrity of the url property and it's state. @kopiro can you do that? Quoting @sgtcoolguy's good tutorial here:

@chmiiller regarding Hans' comment about our unit tests. We maintain a test suite now that we use against PRs. The suite itself is at https://github.com/appcelerator/titanium-mobile-mocha-suite

In particular, it'd be good to add a unit test for Ti.UI.Label.setMaxLines in https://github.com/appcelerator/titanium-mobile-mocha-suite/blob/master/Resources/ti.ui.label.test.js

To do so, clone that repo down locally. Copy titanium-mobile-mocha-suite/Resources/ti.ui.label.test.js to titanium_mobile/tests/Resources/ti.ui.label.test.js and then edit it to add a test for maxLines. (Anything in the titanium_mobile/tests folder acts as an "override" for the test suite. So you could also cheat and copy the app.js and comment out all the other suites other than ti.ui.label.test.)

You can run the test suite with that change locally by doing:

cd titanium_mobile/build
npm install
node scons.js cleanbuild ios # builds and installs ios-only SDK as 6.1.0 in your system
node scons.js test iOS # runs the unit tests against iOS

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

3 participants