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-25048] Android: Implement Ti.UI.ShortcutItem for dynamic application shortcuts #9426

Merged
merged 14 commits into from Oct 16, 2018

Conversation

garymathews
Copy link
Contributor

@garymathews garymathews commented Sep 13, 2017

  • Implement Titanium.UI.ShortcutItem for dynamic application shortcuts
TEST CASE
REQUIREMENTS
  • Android 7.1 or later
  • Launcher that supports application shortcuts (Google Now Launcher or Google Pixel Launcher)
var win = Ti.UI.createWindow({backgroundColor: 'grey'}),
    btn = Ti.UI.createButton({title: 'TOGGLE SHORTCUT'}),
    shortcut = Ti.UI.createShortcutItem({
        id: 'test_shortcut',
        icon: Ti.Android.R.drawable.ic_dialog_info,
        title: 'TEST',
        description: 'DESCRIPTION'
    });

win.backgroundColor = shortcut.visible ? 'green' : 'red';

Ti.App.addEventListener('shortcutitemclick',(e) => {
    alert('id: ' + e.id);
    win.backgroundColor = 'blue';
});

btn.addEventListener('click', () => {
    if (shortcut.visible) {
        win.backgroundColor = 'red';
        shortcut.hide();
    } else {
        win.backgroundColor = 'green';
        shortcut.show();
    }
});

win.add(btn);
win.open();

JIRA Ticket

@hansemannn
Copy link
Collaborator

Mhh, it's very different from how iOS handle shortcut-items. iOS places them on the home-screen and does not allow any hide/show interactions with it. Except for the "title" property, there is not mich parity right now. Also, it should be "ShortcutItem" to match the API. We may want to write up a wiki-page to discuss the API design before taking this into further review. Let's discuss this on Teams!

@garymathews
Copy link
Contributor Author

garymathews commented Sep 13, 2017

@hansemannn Maybe I should rename hide() and show() to add() and remove().

iOS has this:

add
UIApplication.sharedApplication().shortcutItems?.append(...);
remove
UIApplication.sharedApplication().shortcutItems?.removeAtIndex(...);

The only creation parameter iOS doesn't have is intent, which is why I'm thinking about moving to either an event or callback instead. There is parity with everything else:

CREATION PARAMETERS

I'm trying not to abide by either platforms naming scheme since it's our Titanium API and should not be platform specific.

@hansemannn
Copy link
Collaborator

hansemannn commented Sep 13, 2017

Ahh, now it's making sense! Using "add" and "remove" would be awesome! And being able to move the intent out would also be brilliant. For the properties, we may want to use:

  • identifier
  • icon
  • title
  • subtitle

What do you think? And yeah, the Ti.App.iOS events may also need to move. Do you have shortcutitemclick ? And can we use ShortcutItem? Or even a whole different name?

@garymathews garymathews changed the title [TIMOB-25048] Android: Implement Ti.UI.Shortcut for dynamic application shortcuts [TIMOB-25048] Android: Implement Ti.UI.ShortcutItem for dynamic application shortcuts Oct 16, 2017
@garymathews
Copy link
Contributor Author

@hansemannn Updated PR.

@garymathews garymathews force-pushed the TIMOB-25048 branch 2 times, most recently from b0de39b to 6b43702 Compare October 16, 2017 20:54
@build
Copy link
Contributor

build commented Oct 16, 2017

Fails
🚫

🔬 There are library changes, but no changes to the unit tests. That's OK as long as you're refactoring existing code, but will require an admin to merge this PR. Please see README.md#unit-tests for docs on unit testing.

🚫

Tests have failed, see below for more information.

Messages
📖

💾 Here's the generated SDK zipfile.

Tests:

Classname Name Time Error
android.Titanium.UI.ListView .refreshControl (Basic) 60.13 Error: timeout of 60000ms exceeded

Generated by 🚫 dangerJS

Copy link
Contributor

@ypbnv ypbnv left a comment

Choose a reason for hiding this comment

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

LGTM! Just two minor things - check in comments.

import org.appcelerator.kroll.common.Log;
import org.appcelerator.titanium.TiApplication;
import org.appcelerator.titanium.TiC;
import org.appcelerator.titanium.proxy.IntentProxy;
Copy link
Contributor

Choose a reason for hiding this comment

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

Unused import.


var win = Ti.UI.createWindow({backgroundColor: 'grey'}),
btn = Ti.UI.createButton({title: 'TOGGLE SHORTCUT'}),
shortcut = Ti.UI.createShortcut({
Copy link
Contributor

Choose a reason for hiding this comment

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

Typo in Ti.UI.createShortcut, should be Ti.UI.createShortcutItem.

@garymathews
Copy link
Contributor Author

@hansemannn @ypbnv Updated PR

@hansemannn
Copy link
Collaborator

@garymathews Will do the iOS change shortly after 7.0.0.

platforms: [android, iphone, ipad]

properties:
- name: id
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we use identifier here? It's a more common property name on Titanium. iOS right now uses itemtype, which I am okay with changing it for parity.

- name: icon
summary: Shortcut icon.
description: This can be set as a resource id or file path.
type: [String, Number]
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 add iOS-specific notes here, because it can be a Ti.Contacts.Person instance, a constant (SHORTCUT_ICON_TYPE_*) or name of an image (e.g. settings for an image named settings.png).

@garymathews garymathews force-pushed the TIMOB-25048 branch 2 times, most recently from 88f215a to 6288401 Compare May 17, 2018 21:54
@lokeshchdhry
Copy link
Contributor

@garymathews , Clicking the shortcutitem opens up the app but I see that the shortcutitemclick event is fired 2 times.

@sgtcoolguy sgtcoolguy modified the milestones: 7.3.0, 7.4.0 May 23, 2018
@build
Copy link
Contributor

build commented Aug 13, 2018

Messages
📖

💾 Here's the generated SDK zipfile.

Generated by 🚫 dangerJS

@garymathews
Copy link
Contributor Author

@lokeshchdhry Updated PR

@jquick-axway jquick-axway modified the milestones: 7.4.0, 7.5.0 Aug 24, 2018
@lokeshchdhry
Copy link
Contributor

lokeshchdhry commented Oct 16, 2018

FR Passed.

Studio Ver: 5.1.2.201810080801
SDK Ver: 7.5.0 local build
OS Ver: 10.14
Xcode Ver: Xcode 10.0
Appc NPM: 4.2.14-2
Appc CLI: 7.0.6
Daemon Ver: 1.1.3
Ti CLI Ver: 5.1.1
Alloy Ver: 1.13.2
Node Ver: 8.9.1
NPM Ver: 5.5.1
Java Ver: 10.0.2
Devices: ⇨ google Nexus 5 (Android 6.0.1)
⇨ google Pixel (Android 9)
Emulator: ⇨ Android 8.1.0

@lokeshchdhry lokeshchdhry merged commit eba461f into tidev:master Oct 16, 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

8 participants