From 6f674f21f652699da0d0b9855fcfa061f713586f Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Mon, 25 Dec 2017 13:27:23 -0700 Subject: [PATCH 1/3] 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", From 2815c576c1c2758ec6fce69775b92ff99b6e505e Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Mon, 25 Dec 2017 13:29:30 -0700 Subject: [PATCH 2/3] Ignore the default value when calculating if notifications are enabled This is to make the "Enable desktop notifications" checkbox accurately reflect the user's preference. The device-level setting is still respected. Signed-off-by: Travis Ralston --- src/settings/SettingsStore.js | 12 ++++++------ src/settings/controllers/NotificationControllers.js | 4 ++-- src/settings/controllers/SettingController.js | 4 +++- 3 files changed, 11 insertions(+), 9 deletions(-) diff --git a/src/settings/SettingsStore.js b/src/settings/SettingsStore.js index d93a48005d..a1b88fb0c2 100644 --- a/src/settings/SettingsStore.js +++ b/src/settings/SettingsStore.js @@ -222,9 +222,9 @@ export default class SettingsStore { if (explicit) { const handler = handlers[level]; - if (!handler) return SettingsStore._tryControllerOverride(settingName, level, roomId, null); + if (!handler) return SettingsStore._tryControllerOverride(settingName, level, roomId, null, null); const value = handler.getValue(settingName, roomId); - return SettingsStore._tryControllerOverride(settingName, level, roomId, value); + return SettingsStore._tryControllerOverride(settingName, level, roomId, value, level); } for (let i = minIndex; i < levelOrder.length; i++) { @@ -234,17 +234,17 @@ export default class SettingsStore { const value = handler.getValue(settingName, roomId); if (value === null || value === undefined) continue; - return SettingsStore._tryControllerOverride(settingName, level, roomId, value); + return SettingsStore._tryControllerOverride(settingName, level, roomId, value, levelOrder[i]); } - return SettingsStore._tryControllerOverride(settingName, level, roomId, null); + return SettingsStore._tryControllerOverride(settingName, level, roomId, null, null); } - static _tryControllerOverride(settingName, level, roomId, calculatedValue) { + static _tryControllerOverride(settingName, level, roomId, calculatedValue, calculatedAtLevel) { const controller = SETTINGS[settingName].controller; if (!controller) return calculatedValue; - const actualValue = controller.getValueOverride(level, roomId, calculatedValue); + const actualValue = controller.getValueOverride(level, roomId, calculatedValue, calculatedAtLevel); if (actualValue !== undefined && actualValue !== null) return actualValue; return calculatedValue; } diff --git a/src/settings/controllers/NotificationControllers.js b/src/settings/controllers/NotificationControllers.js index 9dcf78e26b..e78b67e847 100644 --- a/src/settings/controllers/NotificationControllers.js +++ b/src/settings/controllers/NotificationControllers.js @@ -35,11 +35,11 @@ function isMasterRuleEnabled() { } export class NotificationsEnabledController extends SettingController { - getValueOverride(level, roomId, calculatedValue) { + getValueOverride(level, roomId, calculatedValue, calculatedAtLevel) { const Notifier = require('../../Notifier'); // avoids cyclical references if (!Notifier.isPossible()) return false; - if (calculatedValue === null) { + if (calculatedValue === null || calculatedAtLevel === "default") { return isMasterRuleEnabled(); } diff --git a/src/settings/controllers/SettingController.js b/src/settings/controllers/SettingController.js index a91b616da9..0ebe0042e6 100644 --- a/src/settings/controllers/SettingController.js +++ b/src/settings/controllers/SettingController.js @@ -31,9 +31,11 @@ export default class SettingController { * @param {String} roomId The room ID, may be null. * @param {*} calculatedValue The value that the handlers think the setting should be, * may be null. + * @param {string} calculatedAtLevel The level for which the calculated value was + * calculated at. May be null. * @return {*} The value that should be used, or null if no override is applicable. */ - getValueOverride(level, roomId, calculatedValue) { + getValueOverride(level, roomId, calculatedValue, calculatedAtLevel) { return null; // no override } From e8392dfa0093466a708a3a14ba8da576469e6c60 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Mon, 25 Dec 2017 13:39:32 -0700 Subject: [PATCH 3/3] Have /tint use the primary color as the secondary if no secondary was given This is to make the color scheme actually apply itself now that the secondary color is not optional. In order to preserve it being optional in the command, we'll use the primary color as the secondary color as it has no major visual impact. Signed-off-by: Travis Ralston --- src/SlashCommands.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/SlashCommands.js b/src/SlashCommands.js index 344bac1ddb..4e711d6f44 100644 --- a/src/SlashCommands.js +++ b/src/SlashCommands.js @@ -96,6 +96,8 @@ const commands = { colorScheme.primary_color = matches[1]; if (matches[4]) { colorScheme.secondary_color = matches[4]; + } else { + colorScheme.secondary_color = colorScheme.primary_color; } return success( SettingsStore.setValue("roomColor", roomId, SettingLevel.ROOM_ACCOUNT, colorScheme),