-
-
Notifications
You must be signed in to change notification settings - Fork 832
Fix various issues surrounding granular settings to date #1613
Changes from 8 commits
022e40a
cf8ff6a
f141ee1
10a1d9c
5976fb2
fb1f20b
d0a0a9c
3e13a91
f62b04c
52e2c98
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -613,8 +613,7 @@ module.exports = React.createClass({ | |
|
||
onLanguageChange: function(newLang) { | ||
if(this.state.language !== newLang) { | ||
// We intentionally promote this to the account level at this point | ||
SettingsStore.setValue("language", null, SettingLevel.ACCOUNT, newLang); | ||
SettingsStore.setValue("language", null, SettingLevel.DEVICE, newLang); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. for the inevitable question: I have no idea why this got promoted to an account level setting. It's always been a local setting. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. hah, i love the idea of changing language on one device magically synchronising over all your accounts :D There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 'love' in a "aaaaaargh no" kinda way |
||
this.setState({ | ||
language: newLang, | ||
}); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -15,12 +15,35 @@ limitations under the License. | |
*/ | ||
|
||
import SettingController from "./SettingController"; | ||
import MatrixClientPeg from '../../MatrixClientPeg'; | ||
|
||
// XXX: This feels wrong. | ||
import PushProcessor from "matrix-js-sdk/lib/pushprocessor"; | ||
|
||
function isMasterRuleEnabled() { | ||
// Return the value of the master push rule as a default | ||
const processor = new PushProcessor(MatrixClientPeg.get()); | ||
const masterRule = processor.getPushRuleById(".m.rule.master"); | ||
|
||
if (!masterRule) { | ||
console.warn("No master push rule! Notifications are disabled for this user."); | ||
return false; | ||
} | ||
|
||
// Why enabled == false means "enabled" is beyond me. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Because the masterRule acts to disable push, apparently. |
||
return !masterRule.enabled; | ||
} | ||
|
||
export class NotificationsEnabledController extends SettingController { | ||
getValueOverride(level, roomId, calculatedValue) { | ||
const Notifier = require('../../Notifier'); // avoids cyclical references | ||
if (!Notifier.isPossible()) return false; | ||
|
||
if (calculatedValue === null) { | ||
return isMasterRuleEnabled(); | ||
} | ||
|
||
return calculatedValue && Notifier.isPossible(); | ||
return calculatedValue; | ||
} | ||
|
||
onChange(level, roomId, newValue) { | ||
|
@@ -35,15 +58,22 @@ export class NotificationsEnabledController extends SettingController { | |
export class NotificationBodyEnabledController extends SettingController { | ||
getValueOverride(level, roomId, calculatedValue) { | ||
const Notifier = require('../../Notifier'); // avoids cyclical references | ||
if (!Notifier.isPossible()) return false; | ||
|
||
if (calculatedValue === null) { | ||
return isMasterRuleEnabled(); | ||
} | ||
|
||
return calculatedValue && Notifier.isEnabled(); | ||
return calculatedValue; | ||
} | ||
} | ||
|
||
export class AudioNotificationsEnabledController extends SettingController { | ||
getValueOverride(level, roomId, calculatedValue) { | ||
const Notifier = require('../../Notifier'); // avoids cyclical references | ||
if (!Notifier.isPossible()) return false; | ||
|
||
return calculatedValue && Notifier.isEnabled(); | ||
// Note: Audio notifications are *not* enabled by default. | ||
return calculatedValue; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
\o/ :D