Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code

Unified Diff: lib/notificationHelper.js

Issue 29326181: Issue 3022 - Implemented new notification type for normal messages (Closed)
Patch Set: Centralized mapping information for types Created Sept. 9, 2015, 3:25 p.m.
Use n/p to move between diff chunks; N/P to move between comments.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « no previous file | notification.js » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: lib/notificationHelper.js
===================================================================
--- a/lib/notificationHelper.js
+++ b/lib/notificationHelper.js
@@ -24,6 +24,13 @@
let {initAntiAdblockNotification} = require("antiadblockInit");
let activeNotification = null;
+let displayMethods = {
Sebastian Noack 2015/09/09 17:38:57 Please use Object.create(null) for objects used as
+ _default: ["popup"],
Sebastian Noack 2015/09/09 17:38:57 There is no reason why this is a prefixed property
+ critical: ["icon", "notification", "popup"],
+ question: ["notification"],
+ normal: ["notification"],
+ information: ["icon", "popup"]
+};
// Chrome on Linux does not fully support chrome.notifications until version 35
// https://code.google.com/p/chromium/issues/detail?id=291485
@@ -42,7 +49,7 @@
function prepareNotificationIconAndPopup()
{
- let animateIcon = (activeNotification.type != "question");
+ let animateIcon = shouldDisplay("icon", activeNotification.type);
activeNotification.onClicked = function()
{
if (animateIcon)
@@ -135,7 +142,7 @@
return;
activeNotification = notification;
- if (activeNotification.type == "critical" || activeNotification.type == "question")
+ if (shouldDisplay("notification", activeNotification.type))
{
let texts = NotificationStorage.getLocalizedTexts(notification);
let title = texts.title || "";
@@ -239,4 +246,19 @@
return activeNotification;
};
+/**
+ * Determines whether a given display method should be used for a
+ * specified notification type.
+ *
+ * @param {string} method Display method: icon, notification or popup
+ * @param {string} notificationType
+ * @return {boolean}
+ */
+exports.shouldDisplay = shouldDisplay;
+function shouldDisplay(method, notificationType)
+{
+ let methods = displayMethods[notificationType] || displayMethods._default;
+ return methods.indexOf(method) > -1;
Sebastian Noack 2015/09/09 17:38:57 If you use objects for these "lists", you can use
+}
+
NotificationStorage.addShowListener(showNotification);
« no previous file with comments | « no previous file | notification.js » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld