From 844ee0a3f436c9143aa31f43e80176f939d08b14 Mon Sep 17 00:00:00 2001 From: Zhuo Lu Date: Mon, 15 Jan 2018 20:50:03 -0800 Subject: [PATCH 01/11] Accept additional notification actions Change to the existing API definition: The first action with type `button` seen will be displayed on the notification, the rest listed as additional actions (shown when holding down on the primary action button) --- brightray/browser/mac/cocoa_notification.h | 5 ++- brightray/browser/mac/cocoa_notification.mm | 40 +++++++++++++++++-- .../mac/notification_center_delegate.mm | 11 +++-- 3 files changed, 48 insertions(+), 8 deletions(-) diff --git a/brightray/browser/mac/cocoa_notification.h b/brightray/browser/mac/cocoa_notification.h index 9e7b55fabfddf..3f27039328de8 100644 --- a/brightray/browser/mac/cocoa_notification.h +++ b/brightray/browser/mac/cocoa_notification.h @@ -9,6 +9,7 @@ #include #include +#include #include "base/mac/scoped_nsobject.h" #include "brightray/browser/notification.h" @@ -28,6 +29,7 @@ class CocoaNotification : public Notification { void NotificationDisplayed(); void NotificationReplied(const std::string& reply); void NotificationButtonClicked(); + void NotificationAdditionalActionClicked(NSUserNotificationAction* action); NSUserNotification* notification() const { return notification_; } @@ -35,7 +37,8 @@ class CocoaNotification : public Notification { void LogAction(const char* action); base::scoped_nsobject notification_; - int action_index_; + std::map additional_action_indices_; + unsigned action_index_; DISALLOW_COPY_AND_ASSIGN(CocoaNotification); }; diff --git a/brightray/browser/mac/cocoa_notification.mm b/brightray/browser/mac/cocoa_notification.mm index 94b8f93fdc3f6..e1a8092d2acea 100644 --- a/brightray/browser/mac/cocoa_notification.mm +++ b/brightray/browser/mac/cocoa_notification.mm @@ -59,14 +59,31 @@ [notification_ setHasActionButton:false]; int i = 0; + action_index_ = UINT_MAX; + NSMutableArray* additionalActions = [[[NSMutableArray alloc] init] autorelease]; for (const auto& action : options.actions) { if (action.type == base::ASCIIToUTF16("button")) { - [notification_ setHasActionButton:true]; - [notification_ setActionButtonTitle:base::SysUTF16ToNSString(action.text)]; - action_index_ = i; + if (action_index_ == UINT_MAX) { + // First button observed is the displayed action + [notification_ setHasActionButton:true]; + [notification_ setActionButtonTitle:base::SysUTF16ToNSString(action.text)]; + action_index_ = i; + } else { + // All of the rest are appended to the list of additional actions + NSString* actionIdentifier = [NSString stringWithFormat:@"%@Action%d", identifier, i]; + NSUserNotificationAction* notificationAction = + [NSUserNotificationAction actionWithIdentifier:actionIdentifier + title:base::SysUTF16ToNSString(action.text)]; + [additionalActions addObject:notificationAction]; + additional_action_indices_.insert(std::make_pair(base::SysNSStringToUTF8(actionIdentifier), i)); + } } i++; } + if ([additionalActions count] > 0 && + [notification_ respondsToSelector:@selector(setAdditionalActions:)]) { + [notification_ setAdditionalActions:additionalActions]; // Requires macOS 10.10 + } if (options.has_reply) { [notification_ setResponsePlaceholder:base::SysUTF16ToNSString(options.reply_placeholder)]; @@ -108,6 +125,23 @@ this->LogAction("button clicked"); } +void CocoaNotification::NotificationAdditionalActionClicked(NSUserNotificationAction* action) { + if (delegate()) { + unsigned index = action_index_; + std::string identifier = base::SysNSStringToUTF8(action.identifier); + for (const auto& it : additional_action_indices_) { + if (it.first == identifier) { + index = it.second; + break; + } + } + + delegate()->NotificationAction(index); + } + + this->LogAction("button clicked"); +} + void CocoaNotification::LogAction(const char* action) { if (getenv("ELECTRON_DEBUG_NOTIFICATIONS")) { NSString* identifier = [notification_ valueForKey:@"identifier"]; diff --git a/brightray/browser/mac/notification_center_delegate.mm b/brightray/browser/mac/notification_center_delegate.mm index 6c9fb7f277dd2..e4c9ab8a63708 100644 --- a/brightray/browser/mac/notification_center_delegate.mm +++ b/brightray/browser/mac/notification_center_delegate.mm @@ -34,12 +34,15 @@ - (void)userNotificationCenter:(NSUserNotificationCenter*)center } if (notification) { - if (notif.activationType == NSUserNotificationActivationTypeReplied) { - notification->NotificationReplied([notif.response.string UTF8String]); + // Ref: https://developer.apple.com/documentation/foundation/nsusernotificationactivationtype?language=objc + if (notif.activationType == NSUserNotificationActivationTypeContentsClicked) { + notification->NotificationClicked(); } else if (notif.activationType == NSUserNotificationActivationTypeActionButtonClicked) { notification->NotificationButtonClicked(); - } else if (notif.activationType == NSUserNotificationActivationTypeContentsClicked) { - notification->NotificationClicked(); + } else if (notif.activationType == NSUserNotificationActivationTypeReplied) { + notification->NotificationReplied([notif.response.string UTF8String]); + } else if (notif.activationType == NSUserNotificationActivationTypeAdditionalActionClicked) { + notification->NotificationAdditionalActionClicked([notif additionalActivationAction]); } } } From 09d51f0a3d5f9a0c1a7a3bb25ee62470ce742637 Mon Sep 17 00:00:00 2001 From: Zhuo Lu Date: Mon, 15 Jan 2018 21:07:27 -0800 Subject: [PATCH 02/11] Fix include order --- brightray/browser/mac/cocoa_notification.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/brightray/browser/mac/cocoa_notification.h b/brightray/browser/mac/cocoa_notification.h index 3f27039328de8..3c2aa367ea2db 100644 --- a/brightray/browser/mac/cocoa_notification.h +++ b/brightray/browser/mac/cocoa_notification.h @@ -7,9 +7,9 @@ #import +#include #include #include -#include #include "base/mac/scoped_nsobject.h" #include "brightray/browser/notification.h" From 3ee3e9a3e8763697b782cfe6c9b1b650bb66381f Mon Sep 17 00:00:00 2001 From: Zhuo Lu Date: Tue, 16 Jan 2018 01:36:42 -0800 Subject: [PATCH 03/11] Fix typo --- docs/api/structures/notification-action.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/api/structures/notification-action.md b/docs/api/structures/notification-action.md index a2cf4e078a916..62bc5bce3f9bc 100644 --- a/docs/api/structures/notification-action.md +++ b/docs/api/structures/notification-action.md @@ -15,6 +15,6 @@ In order for extra notification buttons to work on macOS your app must meet the following criteria. * App is signed -* App has it's `NSUserNotificationAlertStyle` set to `alert` in the `info.plist`. +* App has it's `NSUserNotificationAlertStyle` set to `alert` in the `Info.plist`. If either of these requirements are not met the button simply won't appear. From 31baafab3be1f814cdc6ac281a644e8ae223166a Mon Sep 17 00:00:00 2001 From: Zhuo Lu Date: Tue, 16 Jan 2018 18:59:30 -0800 Subject: [PATCH 04/11] NSUserNotification should respond NSUserNotification is expected to responsd to `@selector(setContentImage:)` with macOS ^10.9 --- brightray/browser/mac/cocoa_notification.mm | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/brightray/browser/mac/cocoa_notification.mm b/brightray/browser/mac/cocoa_notification.mm index e1a8092d2acea..d669577badf5b 100644 --- a/brightray/browser/mac/cocoa_notification.mm +++ b/brightray/browser/mac/cocoa_notification.mm @@ -41,8 +41,7 @@ LOG(INFO) << "Notification created (" << [identifier UTF8String] << ")"; } - if ([notification_ respondsToSelector:@selector(setContentImage:)] && - !options.icon.drawsNothing()) { + if (!options.icon.drawsNothing()) { NSImage* image = skia::SkBitmapToNSImageWithColorSpace( options.icon, base::mac::GetGenericRGBColorSpace()); [notification_ setContentImage:image]; From 1e1087abbba3347dfc327c08a10379db9bee313a Mon Sep 17 00:00:00 2001 From: Zhuo Lu Date: Tue, 16 Jan 2018 18:59:58 -0800 Subject: [PATCH 05/11] Simplify formatting --- brightray/browser/mac/cocoa_notification.mm | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/brightray/browser/mac/cocoa_notification.mm b/brightray/browser/mac/cocoa_notification.mm index d669577badf5b..17f556efb9ddc 100644 --- a/brightray/browser/mac/cocoa_notification.mm +++ b/brightray/browser/mac/cocoa_notification.mm @@ -29,13 +29,12 @@ void CocoaNotification::Show(const NotificationOptions& options) { notification_.reset([[NSUserNotification alloc] init]); - NSString* identifier = [NSString stringWithFormat:@"%s%d", "ElectronNotification", g_identifier_]; + NSString* identifier = [NSString stringWithFormat:@"ElectronNotification%d", g_identifier_++]; [notification_ setTitle:base::SysUTF16ToNSString(options.title)]; [notification_ setSubtitle:base::SysUTF16ToNSString(options.subtitle)]; [notification_ setInformativeText:base::SysUTF16ToNSString(options.msg)]; [notification_ setIdentifier:identifier]; - g_identifier_++; if (getenv("ELECTRON_DEBUG_NOTIFICATIONS")) { LOG(INFO) << "Notification created (" << [identifier UTF8String] << ")"; From 75b990faffdc7338c6571ff286f904f7e7167bdd Mon Sep 17 00:00:00 2001 From: Zhuo Lu Date: Tue, 16 Jan 2018 19:00:39 -0800 Subject: [PATCH 06/11] Use std::string::empty to check --- brightray/browser/mac/cocoa_notification.mm | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/brightray/browser/mac/cocoa_notification.mm b/brightray/browser/mac/cocoa_notification.mm index 17f556efb9ddc..73bf44a05b9c8 100644 --- a/brightray/browser/mac/cocoa_notification.mm +++ b/brightray/browser/mac/cocoa_notification.mm @@ -48,10 +48,10 @@ if (options.silent) { [notification_ setSoundName:nil]; - } else if (options.sound != nil) { - [notification_ setSoundName:base::SysUTF16ToNSString(options.sound)]; - } else { + } else if (options.sound.empty()) { [notification_ setSoundName:NSUserNotificationDefaultSoundName]; + } else { + [notification_ setSoundName:base::SysUTF16ToNSString(options.sound)]; } [notification_ setHasActionButton:false]; From 0b7a629a4101996da23e42f0bcfec2a70a5c6101 Mon Sep 17 00:00:00 2001 From: Zhuo Lu Date: Tue, 16 Jan 2018 19:07:54 -0800 Subject: [PATCH 07/11] Overload method for naming consistency --- brightray/browser/mac/cocoa_notification.h | 4 ++-- brightray/browser/mac/cocoa_notification.mm | 4 ++-- brightray/browser/mac/notification_center_delegate.mm | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/brightray/browser/mac/cocoa_notification.h b/brightray/browser/mac/cocoa_notification.h index 3c2aa367ea2db..1cdd7e33b423b 100644 --- a/brightray/browser/mac/cocoa_notification.h +++ b/brightray/browser/mac/cocoa_notification.h @@ -28,8 +28,8 @@ class CocoaNotification : public Notification { void NotificationDisplayed(); void NotificationReplied(const std::string& reply); - void NotificationButtonClicked(); - void NotificationAdditionalActionClicked(NSUserNotificationAction* action); + void NotificationActivated(); + void NotificationActivated(NSUserNotificationAction* action); NSUserNotification* notification() const { return notification_; } diff --git a/brightray/browser/mac/cocoa_notification.mm b/brightray/browser/mac/cocoa_notification.mm index 73bf44a05b9c8..7d0786266108a 100644 --- a/brightray/browser/mac/cocoa_notification.mm +++ b/brightray/browser/mac/cocoa_notification.mm @@ -116,14 +116,14 @@ this->LogAction("replied to"); } -void CocoaNotification::NotificationButtonClicked() { +void CocoaNotification::NotificationActivated() { if (delegate()) delegate()->NotificationAction(action_index_); this->LogAction("button clicked"); } -void CocoaNotification::NotificationAdditionalActionClicked(NSUserNotificationAction* action) { +void CocoaNotification::NotificationActivated(NSUserNotificationAction* action) { if (delegate()) { unsigned index = action_index_; std::string identifier = base::SysNSStringToUTF8(action.identifier); diff --git a/brightray/browser/mac/notification_center_delegate.mm b/brightray/browser/mac/notification_center_delegate.mm index e4c9ab8a63708..1cb3dc9a33d33 100644 --- a/brightray/browser/mac/notification_center_delegate.mm +++ b/brightray/browser/mac/notification_center_delegate.mm @@ -38,11 +38,11 @@ - (void)userNotificationCenter:(NSUserNotificationCenter*)center if (notif.activationType == NSUserNotificationActivationTypeContentsClicked) { notification->NotificationClicked(); } else if (notif.activationType == NSUserNotificationActivationTypeActionButtonClicked) { - notification->NotificationButtonClicked(); + notification->NotificationActivated(); } else if (notif.activationType == NSUserNotificationActivationTypeReplied) { notification->NotificationReplied([notif.response.string UTF8String]); } else if (notif.activationType == NSUserNotificationActivationTypeAdditionalActionClicked) { - notification->NotificationAdditionalActionClicked([notif additionalActivationAction]); + notification->NotificationActivated([notif additionalActivationAction]); } } } From 69e65e0d5e94f3443c1b4caa1eb6a5fec45c0ade Mon Sep 17 00:00:00 2001 From: Zhuo Lu Date: Tue, 16 Jan 2018 19:11:04 -0800 Subject: [PATCH 08/11] Update NotificationAction doc --- docs/api/structures/notification-action.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/api/structures/notification-action.md b/docs/api/structures/notification-action.md index 62bc5bce3f9bc..fafeff6b71563 100644 --- a/docs/api/structures/notification-action.md +++ b/docs/api/structures/notification-action.md @@ -7,7 +7,7 @@ | Action Type | Platform Support | Usage of `text` | Default `text` | Limitations | |-------------|------------------|-----------------|----------------|-------------| -| `button` | macOS | Used as the label for the button | "Show" | Maximum of one button, if multiple are provided only the last is used. This action is also incompatible with `hasReply` and will be ignored if `hasReply` is `true`. | +| `button` | macOS | Used as the label for the button | "Show" | Only the first one is used. If multiple are provided, those beyond the first will be listed as additional actions (displayed when mouse active over the action button). Any of such action also is incompatible with `hasReply` and will be ignored if `hasReply` is `true`. | ### Button support on macOS From bf11b09d35fb9a47715bb6c07fb6b8538beabe76 Mon Sep 17 00:00:00 2001 From: Zhuo Lu Date: Tue, 16 Jan 2018 19:13:59 -0800 Subject: [PATCH 09/11] Mention change of default value scenario in doc --- docs/api/structures/notification-action.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/api/structures/notification-action.md b/docs/api/structures/notification-action.md index fafeff6b71563..2511d168507f5 100644 --- a/docs/api/structures/notification-action.md +++ b/docs/api/structures/notification-action.md @@ -7,7 +7,7 @@ | Action Type | Platform Support | Usage of `text` | Default `text` | Limitations | |-------------|------------------|-----------------|----------------|-------------| -| `button` | macOS | Used as the label for the button | "Show" | Only the first one is used. If multiple are provided, those beyond the first will be listed as additional actions (displayed when mouse active over the action button). Any of such action also is incompatible with `hasReply` and will be ignored if `hasReply` is `true`. | +| `button` | macOS | Used as the label for the button | "Show" (if first of such `button`, otherwise empty) | Only the first one is used. If multiple are provided, those beyond the first will be listed as additional actions (displayed when mouse active over the action button). Any of such action also is incompatible with `hasReply` and will be ignored if `hasReply` is `true`. | ### Button support on macOS From d05791c1acb6249bb5db66af1aa232a7c1732bb6 Mon Sep 17 00:00:00 2001 From: Zhuo Lu Date: Tue, 16 Jan 2018 19:18:39 -0800 Subject: [PATCH 10/11] Tweak wording --- docs/api/structures/notification-action.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/api/structures/notification-action.md b/docs/api/structures/notification-action.md index 2511d168507f5..88918cb0eb5f3 100644 --- a/docs/api/structures/notification-action.md +++ b/docs/api/structures/notification-action.md @@ -7,7 +7,7 @@ | Action Type | Platform Support | Usage of `text` | Default `text` | Limitations | |-------------|------------------|-----------------|----------------|-------------| -| `button` | macOS | Used as the label for the button | "Show" (if first of such `button`, otherwise empty) | Only the first one is used. If multiple are provided, those beyond the first will be listed as additional actions (displayed when mouse active over the action button). Any of such action also is incompatible with `hasReply` and will be ignored if `hasReply` is `true`. | +| `button` | macOS | Used as the label for the button | "Show" (or a localized string by system default if first of such `button`, otherwise empty) | Only the first one is used. If multiple are provided, those beyond the first will be listed as additional actions (displayed when mouse active over the action button). Any of such action also is incompatible with `hasReply` and will be ignored if `hasReply` is `true`. | ### Button support on macOS From e3b70dd029532a1242b2b2602afcbf8e80acac91 Mon Sep 17 00:00:00 2001 From: Zhuo Lu Date: Tue, 16 Jan 2018 21:18:26 -0800 Subject: [PATCH 11/11] Fix grammar --- docs/api/structures/notification-action.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/api/structures/notification-action.md b/docs/api/structures/notification-action.md index 88918cb0eb5f3..9817502a81590 100644 --- a/docs/api/structures/notification-action.md +++ b/docs/api/structures/notification-action.md @@ -7,7 +7,7 @@ | Action Type | Platform Support | Usage of `text` | Default `text` | Limitations | |-------------|------------------|-----------------|----------------|-------------| -| `button` | macOS | Used as the label for the button | "Show" (or a localized string by system default if first of such `button`, otherwise empty) | Only the first one is used. If multiple are provided, those beyond the first will be listed as additional actions (displayed when mouse active over the action button). Any of such action also is incompatible with `hasReply` and will be ignored if `hasReply` is `true`. | +| `button` | macOS | Used as the label for the button | "Show" (or a localized string by system default if first of such `button`, otherwise empty) | Only the first one is used. If multiple are provided, those beyond the first will be listed as additional actions (displayed when mouse active over the action button). Any such action also is incompatible with `hasReply` and will be ignored if `hasReply` is `true`. | ### Button support on macOS