From dbc72a05bc3918e226db45f91d91c39ab1536c79 Mon Sep 17 00:00:00 2001 From: epriestley Date: Fri, 16 Mar 2018 15:10:26 -0700 Subject: [PATCH] Correct the behavior of "Desktop Only" in Notifications preferences Summary: See . Ref T13102. The "Desktop Only" mode for notifications currently shows both desktop and web notifications. In fact, `JX.Notification` currently has no ability to render notifications as desktop-only. Make this work. Note that many of the variables and parameters here, including `showAnyNotification`, `web_ready`, and `desktop_ready`, are named in an incorrect or misleading way. However, the new behavior appears to be correct. Test Plan: - Emitted test notifications in "No Notifications", "Web Only", "Web and Desktop", and "Desktop" modes. - Saw appropriate notifications appear in the UI. Maniphest Tasks: T13102 Differential Revision: https://secure.phabricator.com/D19233 --- resources/celerity/map.php | 52 +++++++++---------- .../aphlict/behavior-aphlict-listen.js | 3 +- webroot/rsrc/js/core/Notification.js | 13 +++++ 3 files changed, 41 insertions(+), 27 deletions(-) diff --git a/resources/celerity/map.php b/resources/celerity/map.php index 0b49e9f21a..f5a5a57904 100644 --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -10,7 +10,7 @@ return array( 'conpherence.pkg.css' => 'e68cf1fa', 'conpherence.pkg.js' => '15191c65', 'core.pkg.css' => 'c218ed53', - 'core.pkg.js' => '0fabde4f', + 'core.pkg.js' => '8581cd02', 'differential.pkg.css' => '113e692c', 'differential.pkg.js' => 'f6d809c0', 'diffusion.pkg.css' => 'a2d17c7d', @@ -352,7 +352,7 @@ return array( 'rsrc/image/texture/table_header_tall.png' => 'd56b434f', 'rsrc/js/application/aphlict/Aphlict.js' => 'e1d4b11a', 'rsrc/js/application/aphlict/behavior-aphlict-dropdown.js' => 'caade6f2', - 'rsrc/js/application/aphlict/behavior-aphlict-listen.js' => '4cc4f460', + 'rsrc/js/application/aphlict/behavior-aphlict-listen.js' => '599a8f5f', 'rsrc/js/application/aphlict/behavior-aphlict-status.js' => '5e2634b9', 'rsrc/js/application/aphlict/behavior-desktop-notifications-control.js' => '27ca6289', 'rsrc/js/application/calendar/behavior-day-view.js' => '4b3c4443', @@ -445,7 +445,7 @@ return array( 'rsrc/js/core/KeyboardShortcut.js' => '1ae869f2', 'rsrc/js/core/KeyboardShortcutManager.js' => 'c19dd9b9', 'rsrc/js/core/MultirowRowManager.js' => 'b5d57730', - 'rsrc/js/core/Notification.js' => '008faf9c', + 'rsrc/js/core/Notification.js' => '4f774dac', 'rsrc/js/core/Prefab.js' => '77b0ae28', 'rsrc/js/core/ShapedRequest.js' => '7cbe244b', 'rsrc/js/core/TextAreaUtils.js' => '320810c8', @@ -566,7 +566,7 @@ return array( 'javelin-aphlict' => 'e1d4b11a', 'javelin-behavior' => '61cbc29a', 'javelin-behavior-aphlict-dropdown' => 'caade6f2', - 'javelin-behavior-aphlict-listen' => '4cc4f460', + 'javelin-behavior-aphlict-listen' => '599a8f5f', 'javelin-behavior-aphlict-status' => '5e2634b9', 'javelin-behavior-aphront-basic-tokenizer' => 'b3a4b884', 'javelin-behavior-aphront-drag-and-drop-textarea' => '484a6e22', @@ -772,7 +772,7 @@ return array( 'phabricator-keyboard-shortcut-manager' => 'c19dd9b9', 'phabricator-main-menu-view' => '1802a242', 'phabricator-nav-view-css' => 'a9e3e6d5', - 'phabricator-notification' => '008faf9c', + 'phabricator-notification' => '4f774dac', 'phabricator-notification-css' => '457861ec', 'phabricator-notification-menu-css' => '10685bd4', 'phabricator-object-selector-css' => '85ee8ce6', @@ -894,13 +894,6 @@ return array( 'javelin-typeahead-preloaded-source', 'javelin-util', ), - '008faf9c' => array( - 'javelin-install', - 'javelin-dom', - 'javelin-stratcom', - 'javelin-util', - 'phabricator-notification-css', - ), '013ffff9' => array( 'javelin-install', 'javelin-util', @@ -1259,20 +1252,6 @@ return array( 'javelin-uri', 'phabricator-notification', ), - '4cc4f460' => array( - 'javelin-behavior', - 'javelin-aphlict', - 'javelin-stratcom', - 'javelin-request', - 'javelin-uri', - 'javelin-dom', - 'javelin-json', - 'javelin-router', - 'javelin-util', - 'javelin-leader', - 'javelin-sound', - 'phabricator-notification', - ), '4d863052' => array( 'javelin-dom', 'javelin-util', @@ -1289,6 +1268,13 @@ return array( 'javelin-stratcom', 'javelin-dom', ), + '4f774dac' => array( + 'javelin-install', + 'javelin-dom', + 'javelin-stratcom', + 'javelin-util', + 'phabricator-notification-css', + ), '503e17fd' => array( 'javelin-install', 'javelin-typeahead-source', @@ -1343,6 +1329,20 @@ return array( 'javelin-uri', 'phabricator-file-upload', ), + '599a8f5f' => array( + 'javelin-behavior', + 'javelin-aphlict', + 'javelin-stratcom', + 'javelin-request', + 'javelin-uri', + 'javelin-dom', + 'javelin-json', + 'javelin-router', + 'javelin-util', + 'javelin-leader', + 'javelin-sound', + 'phabricator-notification', + ), '59a7976a' => array( 'javelin-install', 'javelin-dom', diff --git a/webroot/rsrc/js/application/aphlict/behavior-aphlict-listen.js b/webroot/rsrc/js/application/aphlict/behavior-aphlict-listen.js index 24571b5981..ac058bde40 100644 --- a/webroot/rsrc/js/application/aphlict/behavior-aphlict-listen.js +++ b/webroot/rsrc/js/application/aphlict/behavior-aphlict-listen.js @@ -78,7 +78,7 @@ JX.behavior('aphlict-listen', function(config) { JX.Stratcom.invoke('notification-panel-update', null, {}); var response = e.getData(); - if (!response.showAnyNotification) { + if (!response.showAnyNotification && !response.showDesktopNotification) { return; } @@ -86,6 +86,7 @@ JX.behavior('aphlict-listen', function(config) { new JX.Notification() .setContent(JX.$H(response.content)) .setKey(response.primaryObjectPHID) + .setShowAsWebNotification(response.showAnyNotification) .setShowAsDesktopNotification(response.showDesktopNotification) .setTitle(response.title) .setBody(response.body) diff --git a/webroot/rsrc/js/core/Notification.js b/webroot/rsrc/js/core/Notification.js index 13e6cc47ae..67e1e5c910 100644 --- a/webroot/rsrc/js/core/Notification.js +++ b/webroot/rsrc/js/core/Notification.js @@ -27,6 +27,7 @@ JX.install('Notification', { _hideTimer : null, _duration : 12000, _asDesktop : false, + _asWeb : true, _key : null, _title : null, _body : null, @@ -88,6 +89,11 @@ JX.install('Notification', { return this; }, + setShowAsWebNotification: function(mode) { + this._asWeb = mode; + return this; + }, + setShowAsDesktopNotification : function(mode) { this._asDesktop = mode; return this; @@ -242,6 +248,13 @@ JX.install('Notification', { var notifications = []; for (var ii = 0; ii < self._active.length; ii++) { + + // Don't render this notification if it's not configured to show as + // a web notification. + if (!self._active[ii]._asWeb) { + continue; + } + notifications.push(self._active[ii]._getContainer()); if (!(--limit)) { break;