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-25045] Android: Implement Ti.Android.NotificationChannel #9305

Merged
merged 5 commits into from Nov 15, 2017

Conversation

garymathews
Copy link
Contributor

@garymathews garymathews commented Aug 11, 2017

REQUIRES #9274 TO BE MERGED

  • Implement Titanium.Android.NotificationChannel
TEST CASE
var win = Ti.UI.createWindow({title: 'TIMOB-250045', layout: 'vertical'}),
    btn = Ti.UI.createButton({title: 'CREATE NOTIFICATION', height: 50, width: Ti.UI.FILL}),
    id = 100,
    channel = Ti.Android.NotificationManager.createNotificationChannel({
        id: 'my_channel',
        name: 'TEST CHANNEL',
        importance: Ti.Android.IMPORTANCE_DEFAULT
    });

btn.addEventListener('click', function() {
    var n = Ti.Android.createNotification({
            icon: Ti.Android.R.drawable.ic_dialog_info,
            contentTitle: 'TITLE',
            contentText : 'This is a test',
            channelId: channel.getId()
        });
    Ti.Android.NotificationManager.notify(id, n);
});

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

JIRA Ticket

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.

Few docs-questions!

Used with [NotificationChannel](Titanium.Android.NotificationChannel) to specify an importance level.
type: Number
permission: read-only

Copy link
Collaborator

Choose a reason for hiding this comment

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

since: "7.0.0" for all of these? Or is it scheduled for 6.2.0?


- name: vibratePattern
summary: The vibration pattern for notifications posted to this channel.
type: Array
Copy link
Collaborator

Choose a reason for hiding this comment

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

Array of what?

osver: {android: {min: "8.0"}}
description: |
Create a notification channel.
parameters:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Does not need multi-line-wrapping, but it's no big deal.

@hansemannn
Copy link
Collaborator

@garymathews Jenkins is failing because of docs-issues:

[ERROR] ./Titanium/Android/NotificationChannel.yml
Titanium.Android.NotificationChannel: found 2 error(s)!
	Required property "summary" not found
	properties
		id
			Invalid array element: creation-only; possible values: read-only,write-only,read-write

[ERROR] ./Titanium/Android/NotificationManager/NotificationManager.yml
Titanium.Android.NotificationManager: found 1 error(s)!
	methods
		createNotificationChannel
			Required property "summary" not found

@garymathews garymathews force-pushed the TIMOB-25045 branch 3 times, most recently from 3c5700f to 4eb0e78 Compare August 22, 2017 15:06
public static final String PROPERTY_ENABLE_RETURN_KEY = "enableReturnKey";

/**
* @module.api
*/
public static final String PROPERTY_ENABLE_VIBRATION = "enableVibrations";
Copy link
Contributor

Choose a reason for hiding this comment

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

The property string is set to "enableVibrations" with an 's', but you document this property without an 's'. This is a typo, right?

type: Boolean

- name: description
summary: user visible description of this channel.
Copy link
Contributor

Choose a reason for hiding this comment

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

Capitalize "user" in sentence.

type: Boolean

- name: vibratePattern
summary: The vibration pattern for notifications posted to this channel.
Copy link
Contributor

Choose a reason for hiding this comment

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

We should document how this array of numbers work. I believe each array element is a duration in milliseconds, right? Where the even indexed elements (ex: 0, 2, 4, 6, etc.) is the vibration time and the odd indexed elements are how long to wait between vibrations.


- name: lockscreenVisibility
summary: Whether or not notifications posted to this channel are shown on the lockscreen in full or redacted form.
type: Number
Copy link
Contributor

Choose a reason for hiding this comment

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

We should define constants for the "lockscreenVisibility" property:
VISIBILITY_PRIVATE = 0
VISIBILITY_PUBLIC = 1
VISIBILITY_SECRET = -1

Copy link
Contributor

Choose a reason for hiding this comment

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

Or at least document what numbers are supported.

type: Number

- name: id
summary: The channel id specified for the notification channel.
Copy link
Contributor

Choose a reason for hiding this comment

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

Indicate that this is a unique string ID defined by the Titanium developer.

summary: Create a notification channel.
parameters:
- name: parameters
summary: Properties to set on a new object, including any defined by <Titanium.Android.NotificationChannel> except those marked not-creation or read-only.
Copy link
Contributor

Choose a reason for hiding this comment

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

Document that this function will return null on Android OS version older than Android 8.

@garymathews garymathews force-pushed the TIMOB-25045 branch 3 times, most recently from f790134 to 64a5595 Compare September 22, 2017 17:32
@garymathews
Copy link
Contributor Author

@jquick-axway Updated PR

@build
Copy link
Contributor

build commented Sep 22, 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.

Generated by 🚫 dangerJS

Copy link
Contributor

@jquick-axway jquick-axway left a comment

Choose a reason for hiding this comment

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

CR: Pass

@lokeshchdhry
Copy link
Contributor

FR Passed.

Notification channel works as expected along with its properties.
If ran on below android 8.0 it will throw runtime error which is expected :

[ERROR] :  [Nexus 5] TiExceptionHandler: (main) [107728,120517] ----- Titanium Javascript Runtime Error -----
[ERROR] :  [Nexus 5] TiExceptionHandler: (main) [0,120517] - In /app.js:15,32
[ERROR] :  [Nexus 5] TiExceptionHandler: (main) [0,120517] - Message: Uncaught TypeError: Cannot read property 'getId' of null
[ERROR] :  [Nexus 5] TiExceptionHandler: (main) [1,120518] - Source:             channelId: channel.getId()
[ERROR] :  [Nexus 5] V8Exception: Exception occurred at /app.js:15: Uncaught TypeError: Cannot read property 'getId' of null

Studio Ver: 4.10.0.201709271713
SDK Ver: 7.0.0 local build
OS Ver: 10.12.3
Xcode Ver: Xcode 8.3.3
Appc NPM: 4.2.10
Appc CLI: 6.3.0
Ti CLI Ver: 5.0.14
Alloy Ver: 1.10.7
Node Ver: 7.10.1
Java Ver: 1.8.0_101
Devices: ⇨ google Pixel XL --- Android 8.0.0
⇨ google Nexus 5 --- Android 6.0.1

@build build added the android label Nov 15, 2017
@build build added the docs label Nov 15, 2017
@eric34 eric34 merged commit e7387a4 into tidev:master Nov 15, 2017
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

6 participants