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-26312]: iOS 12 Expose new NSUserActivity APIs for Siri Intents #10288

Merged
merged 7 commits into from Aug 28, 2018

Conversation

vijaysingh-axway
Copy link
Contributor

@vijaysingh-axway vijaysingh-axway commented Aug 26, 2018

@build
Copy link
Contributor

build commented Aug 26, 2018

Messages
📖

💾 Here's the generated SDK zipfile.

Generated by 🚫 dangerJS


- (void)deleteSavedUserActivitiesForPersistentIdentifiers:(id)persistentIdentifiers
{
ENSURE_ARRAY(persistentIdentifiers);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Did you actually test all the new API's so far? Since this is a method, you need to unbox the persistentIdentifiers attribute with ENSURE_SINGLE_ARG(persistentIdentifiers, NSArray).

- (void)setEligibleForPrediction:(NSNumber *)value
{
ENSURE_TYPE(value, NSNumber);
ENSURE_UI_THREAD(setEligibleForSearch, value);
Copy link
Collaborator

Choose a reason for hiding this comment

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

The UI-thread should be validated before the type-checks.

return NUMBOOL(NO);
}

return NUMBOOL(_userActivity.isEligibleForPrediction);
Copy link
Collaborator

Choose a reason for hiding this comment

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

We should move all these to native Obj-C 2.0 statements, e.g. @(_userActivity.isEligibleForPrediction) and @(NO). But that should be done with an own PR and an own ticket. Maybe we can deprecate the old macros in SDK 8.0.0?.

description: |
To donate a user activity to Siri Shortcuts, set eligibleForPrediction to 'true' and make the
user activity current. To make the user activity current, call the becomeCurrent method of activity.
For more information, see https://developer.apple.com/documentation/sirikit/donating_shortcuts?language=objc.
Copy link
Collaborator

Choose a reason for hiding this comment

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

For more information, see [the Apple docs](https://developer.apple.com/documentation/sirikit/donating_shortcuts?language=objc).

summary: |
A Boolean value that determines whether Siri can suggest the user activity as a shortcut to the user.
description: |
To donate a user activity to Siri Shortcuts, set eligibleForPrediction to 'true' and make the
Copy link
Collaborator

Choose a reason for hiding this comment

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

`true`


#if IS_XCODE_10

- (NSNumber *)eligibleForPrediction
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you make sure all public API's are also in the interface header? It makes it easier to maintain.

- (void)setEligibleForPrediction:(NSNumber *)value;
- (void)setPersistentIdentifier:(NSString *)value;
- (void)deleteSavedUserActivitiesForPersistentIdentifiers:(id)persistentIdentifiers;
- (void)deleteAllSavedUserActivities:(id)unused;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not only the new methods but all methods ;-). Including code-docs and params-descriptions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

All method exposing is fine. Do we use code-docs and params-description in header file in Titanium SDK?

Copy link
Collaborator

Choose a reason for hiding this comment

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

For Ti core-developers, it will help knowing which params are used in the "boxed" arguments. But it looks fine for now.

@sgtcoolguy sgtcoolguy added this to the 7.5.0 milestone Aug 28, 2018
@sgtcoolguy
Copy link
Contributor

7_4_X version is #10292

@hansemannn hansemannn merged commit a464d06 into tidev:master Aug 28, 2018
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

4 participants