From 6f674f21f652699da0d0b9855fcfa061f713586f Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Mon, 25 Dec 2017 13:27:23 -0700 Subject: [PATCH] Hide the notification nag bar after enabling notifications and add a bit of documentation around why the notificationsEnabled setting isn't set here. Signed-off-by: Travis Ralston --- src/Notifier.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/Notifier.js b/src/Notifier.js index 75b698862c..e69bdf4461 100644 --- a/src/Notifier.js +++ b/src/Notifier.js @@ -135,6 +135,10 @@ const Notifier = { const plaf = PlatformPeg.get(); if (!plaf) return; + // Dev note: We don't set the "notificationsEnabled" setting to true here because it is a + // calculated value. It is determined based upon whether or not the master rule is enabled + // and other flags. Setting it here would cause a circular reference. + Analytics.trackEvent('Notifier', 'Set Enabled', enable); // make sure that we persist the current setting audio_enabled setting @@ -168,7 +172,7 @@ const Notifier = { }); // clear the notifications_hidden flag, so that if notifications are // disabled again in the future, we will show the banner again. - this.setToolbarHidden(false); + this.setToolbarHidden(true); } else { dis.dispatch({ action: "notifier_enabled",