From be118d4f13409d235ebced33a92df11770c93625 Mon Sep 17 00:00:00 2001 From: Zhuo Lu Date: Tue, 16 Jan 2018 11:26:41 -0800 Subject: [PATCH 1/4] Make it able to set close button text --- atom/browser/api/atom_api_notification.cc | 14 +++++++++++++- atom/browser/api/atom_api_notification.h | 3 +++ brightray/browser/mac/cocoa_notification.mm | 4 ++++ brightray/browser/notification.h | 1 + 4 files changed, 21 insertions(+), 1 deletion(-) diff --git a/atom/browser/api/atom_api_notification.cc b/atom/browser/api/atom_api_notification.cc index 9bb24df93a6d..b4e94d377b29 100644 --- a/atom/browser/api/atom_api_notification.cc +++ b/atom/browser/api/atom_api_notification.cc @@ -70,6 +70,7 @@ Notification::Notification(v8::Isolate* isolate, opts.Get("hasReply", &has_reply_); opts.Get("actions", &actions_); opts.Get("sound", &sound_); + opts.Get("closeButtonText", &close_button_text_); } } @@ -120,6 +121,10 @@ base::string16 Notification::GetSound() const { return sound_; } +base::string16 Notification::GetCloseButtonText() const { + return close_button_text_; +} + // Setters void Notification::SetTitle(const base::string16& new_title) { title_ = new_title; @@ -154,6 +159,10 @@ void Notification::SetSound(const base::string16& new_sound) { sound_ = new_sound; } +void Notification::SetCloseButtonText(const base::string16& text) { + close_button_text_ = text; +} + void Notification::NotificationAction(int index) { Emit("action", index); } @@ -201,6 +210,7 @@ void Notification::Show() { options.reply_placeholder = reply_placeholder_; options.actions = actions_; options.sound = sound_; + options.close_button_text = close_button_text_; notification_->Show(options); } } @@ -230,7 +240,9 @@ void Notification::BuildPrototype(v8::Isolate* isolate, .SetProperty("actions", &Notification::GetActions, &Notification::SetActions) .SetProperty("sound", &Notification::GetSound, - &Notification::SetSound); + &Notification::SetSound) + .SetProperty("closeButtonText", &Notification::GetCloseButtonText, + &Notification::SetCloseButtonText); } } // namespace api diff --git a/atom/browser/api/atom_api_notification.h b/atom/browser/api/atom_api_notification.h index 50197350bc5e..e8e7a246e840 100644 --- a/atom/browser/api/atom_api_notification.h +++ b/atom/browser/api/atom_api_notification.h @@ -56,6 +56,7 @@ class Notification : public mate::TrackableObject, bool GetHasReply() const; std::vector GetActions() const; base::string16 GetSound() const; + base::string16 GetCloseButtonText() const; // Prop Setters void SetTitle(const base::string16& new_title); @@ -66,6 +67,7 @@ class Notification : public mate::TrackableObject, void SetHasReply(bool new_has_reply); void SetActions(const std::vector& actions); void SetSound(const base::string16& sound); + void SetCloseButtonText(const base::string16& text); private: base::string16 title_; @@ -79,6 +81,7 @@ class Notification : public mate::TrackableObject, bool has_reply_ = false; std::vector actions_; base::string16 sound_; + base::string16 close_button_text_; brightray::NotificationPresenter* presenter_; diff --git a/brightray/browser/mac/cocoa_notification.mm b/brightray/browser/mac/cocoa_notification.mm index 94b8f93fdc3f..98c3b6abe5b8 100644 --- a/brightray/browser/mac/cocoa_notification.mm +++ b/brightray/browser/mac/cocoa_notification.mm @@ -73,6 +73,10 @@ void CocoaNotification::Show(const NotificationOptions& options) { [notification_ setHasReplyButton:true]; } + if (!options.close_button_text.empty()) { + [notification_ setOtherButtonTitle:base::SysUTF16ToNSString(options.close_button_text)]; + } + [NSUserNotificationCenter.defaultUserNotificationCenter deliverNotification:notification_]; } diff --git a/brightray/browser/notification.h b/brightray/browser/notification.h index 69efcff386f5..853888f1cc8e 100644 --- a/brightray/browser/notification.h +++ b/brightray/browser/notification.h @@ -35,6 +35,7 @@ struct NotificationOptions { base::string16 reply_placeholder; base::string16 sound; std::vector actions; + base::string16 close_button_text; }; class Notification { From 0d4c6e327f0379140c7777b9a52733aa4bf8dabc Mon Sep 17 00:00:00 2001 From: Zhuo Lu Date: Tue, 16 Jan 2018 12:05:43 -0800 Subject: [PATCH 2/4] Update Notification API doc --- docs/api/notification.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/api/notification.md b/docs/api/notification.md index 3362fabfea4c..571fc88d7ed5 100644 --- a/docs/api/notification.md +++ b/docs/api/notification.md @@ -39,7 +39,7 @@ Returns `Boolean` - Whether or not desktop notifications are supported on the cu * `replyPlaceholder` String (optional) _macOS_ - The placeholder to write in the inline reply input field. * `sound` String (optional) _macOS_ - The name of the sound file to play when the notification is shown. * `actions` [NotificationAction[]](structures/notification-action.md) (optional) _macOS_ - Actions to add to the notification. Please read the available actions and limitations in the `NotificationAction` documentation. - + * `closeButtonText` String (optional) _macOS_ - A custom title for the close button of an alert. An empty string will cause the default localized text to be used. ### Instance Events From 29f9929703174c56facf471d5471e0cd80a1fab6 Mon Sep 17 00:00:00 2001 From: Zhuo Lu Date: Mon, 22 Jan 2018 10:48:12 -0800 Subject: [PATCH 3/4] Organize code --- atom/browser/api/atom_api_notification.cc | 35 ++++++++++++----------- atom/browser/api/atom_api_notification.h | 12 ++++---- brightray/browser/notification.h | 2 +- 3 files changed, 25 insertions(+), 24 deletions(-) diff --git a/atom/browser/api/atom_api_notification.cc b/atom/browser/api/atom_api_notification.cc index b4e94d377b29..daa46b96cbfd 100644 --- a/atom/browser/api/atom_api_notification.cc +++ b/atom/browser/api/atom_api_notification.cc @@ -105,22 +105,22 @@ bool Notification::GetSilent() const { return silent_; } -base::string16 Notification::GetReplyPlaceholder() const { - return reply_placeholder_; -} - bool Notification::GetHasReply() const { return has_reply_; } -std::vector Notification::GetActions() const { - return actions_; +base::string16 Notification::GetReplyPlaceholder() const { + return reply_placeholder_; } base::string16 Notification::GetSound() const { return sound_; } +std::vector Notification::GetActions() const { + return actions_; +} + base::string16 Notification::GetCloseButtonText() const { return close_button_text_; } @@ -142,12 +142,16 @@ void Notification::SetSilent(bool new_silent) { silent_ = new_silent; } +void Notification::SetHasReply(bool new_has_reply) { + has_reply_ = new_has_reply; +} + void Notification::SetReplyPlaceholder(const base::string16& new_placeholder) { reply_placeholder_ = new_placeholder; } -void Notification::SetHasReply(bool new_has_reply) { - has_reply_ = new_has_reply; +void Notification::SetSound(const base::string16& new_sound) { + sound_ = new_sound; } void Notification::SetActions( @@ -155,10 +159,6 @@ void Notification::SetActions( actions_ = actions; } -void Notification::SetSound(const base::string16& new_sound) { - sound_ = new_sound; -} - void Notification::SetCloseButtonText(const base::string16& text) { close_button_text_ = text; } @@ -232,15 +232,16 @@ void Notification::BuildPrototype(v8::Isolate* isolate, .SetProperty("subtitle", &Notification::GetSubtitle, &Notification::SetSubtitle) .SetProperty("body", &Notification::GetBody, &Notification::SetBody) - .SetProperty("silent", &Notification::GetSilent, &Notification::SetSilent) - .SetProperty("replyPlaceholder", &Notification::GetReplyPlaceholder, - &Notification::SetReplyPlaceholder) + .SetProperty("silent", &Notification::GetSilent, + &Notification::SetSilent) .SetProperty("hasReply", &Notification::GetHasReply, &Notification::SetHasReply) - .SetProperty("actions", &Notification::GetActions, - &Notification::SetActions) + .SetProperty("replyPlaceholder", &Notification::GetReplyPlaceholder, + &Notification::SetReplyPlaceholder) .SetProperty("sound", &Notification::GetSound, &Notification::SetSound) + .SetProperty("actions", &Notification::GetActions, + &Notification::SetActions) .SetProperty("closeButtonText", &Notification::GetCloseButtonText, &Notification::SetCloseButtonText); } diff --git a/atom/browser/api/atom_api_notification.h b/atom/browser/api/atom_api_notification.h index e8e7a246e840..91a5b72c983c 100644 --- a/atom/browser/api/atom_api_notification.h +++ b/atom/browser/api/atom_api_notification.h @@ -52,10 +52,10 @@ class Notification : public mate::TrackableObject, base::string16 GetSubtitle() const; base::string16 GetBody() const; bool GetSilent() const; - base::string16 GetReplyPlaceholder() const; bool GetHasReply() const; - std::vector GetActions() const; + base::string16 GetReplyPlaceholder() const; base::string16 GetSound() const; + std::vector GetActions() const; base::string16 GetCloseButtonText() const; // Prop Setters @@ -63,10 +63,10 @@ class Notification : public mate::TrackableObject, void SetSubtitle(const base::string16& new_subtitle); void SetBody(const base::string16& new_body); void SetSilent(bool new_silent); - void SetReplyPlaceholder(const base::string16& new_reply_placeholder); void SetHasReply(bool new_has_reply); - void SetActions(const std::vector& actions); + void SetReplyPlaceholder(const base::string16& new_reply_placeholder); void SetSound(const base::string16& sound); + void SetActions(const std::vector& actions); void SetCloseButtonText(const base::string16& text); private: @@ -77,10 +77,10 @@ class Notification : public mate::TrackableObject, base::string16 icon_path_; bool has_icon_ = false; bool silent_ = false; - base::string16 reply_placeholder_; bool has_reply_ = false; - std::vector actions_; + base::string16 reply_placeholder_; base::string16 sound_; + std::vector actions_; base::string16 close_button_text_; brightray::NotificationPresenter* presenter_; diff --git a/brightray/browser/notification.h b/brightray/browser/notification.h index 853888f1cc8e..1b81dda5a182 100644 --- a/brightray/browser/notification.h +++ b/brightray/browser/notification.h @@ -28,9 +28,9 @@ struct NotificationOptions { base::string16 subtitle; base::string16 msg; std::string tag; + bool silent; GURL icon_url; SkBitmap icon; - bool silent; bool has_reply; base::string16 reply_placeholder; base::string16 sound; From 38d284590ff0fe77b100e495e12f1c04d7dd1694 Mon Sep 17 00:00:00 2001 From: Zhuo Lu Date: Tue, 6 Feb 2018 22:09:23 -0800 Subject: [PATCH 4/4] Add Notification tests --- spec/api-notification-spec.js | 91 +++++++++++++++++++++++++++++++++++ 1 file changed, 91 insertions(+) create mode 100644 spec/api-notification-spec.js diff --git a/spec/api-notification-spec.js b/spec/api-notification-spec.js new file mode 100644 index 000000000000..f2e9a4f801c1 --- /dev/null +++ b/spec/api-notification-spec.js @@ -0,0 +1,91 @@ +const assert = require('assert') + +const {Notification} = require('electron').remote + +describe('Notification module', () => { + it('inits, gets and sets basic string properties correctly', () => { + const n = new Notification({ + title: 'title', + subtitle: 'subtitle', + body: 'body', + replyPlaceholder: 'replyPlaceholder', + sound: 'sound', + closeButtonText: 'closeButtonText' + }) + + assert.equal(n.title, 'title') + n.title = 'title1' + assert.equal(n.title, 'title1') + + assert.equal(n.subtitle, 'subtitle') + n.subtitle = 'subtitle1' + assert.equal(n.subtitle, 'subtitle1') + + assert.equal(n.body, 'body') + n.body = 'body1' + assert.equal(n.body, 'body1') + + assert.equal(n.replyPlaceholder, 'replyPlaceholder') + n.replyPlaceholder = 'replyPlaceholder1' + assert.equal(n.replyPlaceholder, 'replyPlaceholder1') + + assert.equal(n.sound, 'sound') + n.sound = 'sound1' + assert.equal(n.sound, 'sound1') + + assert.equal(n.closeButtonText, 'closeButtonText') + n.closeButtonText = 'closeButtonText1' + assert.equal(n.closeButtonText, 'closeButtonText1') + }) + + it('inits, gets and sets basic boolean properties correctly', () => { + const n = new Notification({ + silent: true, + hasReply: true + }) + + assert.equal(n.silent, true) + n.silent = false + assert.equal(n.silent, false) + + assert.equal(n.hasReply, true) + n.hasReply = false + assert.equal(n.hasReply, false) + }) + + it('inits, gets and sets actions correctly', () => { + const n = new Notification({ + actions: [ + { + type: 'button', + text: '1' + }, { + type: 'button', + text: '2' + } + ] + }) + + assert.equal(n.actions[0].type, 'button') + assert.equal(n.actions[0].text, '1') + assert.equal(n.actions[1].type, 'button') + assert.equal(n.actions[1].text, '2') + + n.actions = [ + { + type: 'button', + text: '3' + }, { + type: 'button', + text: '4' + } + ] + + assert.equal(n.actions[0].type, 'button') + assert.equal(n.actions[0].text, '3') + assert.equal(n.actions[1].type, 'button') + assert.equal(n.actions[1].text, '4') + }) + + // TODO(sethlu): Find way to test init with notification icon? +})