Skip to content
This repository has been archived by the owner on Sep 4, 2020. It is now read-only.

Add clearAllNotifications() to type definitions #1827

Merged
merged 1 commit into from
Jul 6, 2017

Conversation

szh
Copy link
Contributor

@szh szh commented Jul 4, 2017

Adds the clearAllNotifications() function to the type definitions

Related Issue

Fixes #1536

Motivation and Context

Updates type definitions to match API changes

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

@fredgalvao
Copy link
Collaborator

@macdonst While reviewing the android source code for this, I found that the clearAllNotifications action does not handle any type of failure explicitly:

else if (CLEAR_ALL_NOTIFICATIONS.equals(action)) {
            cordova.getThreadPool().execute(new Runnable() {
                public void run() {
                    Log.v(LOG_TAG, "clearAllNotifications");
                    clearAllNotifications();
                    callbackContext.success();
                }
            });
        }

Would that entail that it will never fail? Should we try/catch it for basic failure handling? Is there any argument to any of those callbacks in any case?

@macdonst
Copy link
Member

macdonst commented Jul 6, 2017

@fredgalvao if this addition to the types file looks good please go ahead and merge it.

WRT clearAllNotifications in Android, I'm not even sure if it can fail but create and issue for it and I'll do a quick look.

@fredgalvao fredgalvao merged commit 2a8e3df into phonegap:master Jul 6, 2017
@fredgalvao
Copy link
Collaborator

@szh Thanks!

@lock
Copy link

lock bot commented Jun 3, 2018

This thread has been automatically locked.

@lock lock bot locked as resolved and limited conversation to collaborators Jun 3, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Typescript definitions are outdated, API needs remirroring
3 participants