Support for enabling email notifications #289
Conversation
Add utility funcs in UserSettingsStore and pass threepids to Notifications so it can do email notif stuff
| @@ -328,12 +328,12 @@ module.exports = React.createClass({ | |||
| ); | |||
| } | |||
| var notification_area; | |||
| if (!MatrixClientPeg.get().isGuest()) { | |||
| if (!MatrixClientPeg.get().isGuest() && this.state.threepids !== undefined) { | |||
There was a problem hiding this comment.
surely we should still show the Notifications settings, even if the user has no threepids?
There was a problem hiding this comment.
If they have no threepids, this should be the empty list.
|
Stealing this from vdh as he's on a boat. @dbkr: LGTM, modulo comments - although I am concerned that we seem to have an asymmetry between getEmailPusher() that just returns the first pusher, whereas addEmailPusher() lets you add multiple ones. What's the intended behaviour here for multiple email pushers? It would be nice to comment the methods to make this clear. |
|
getEmailPusher returns the first pusher for that email address - could make it return an array although it would be an extreme edge case to have to pushers for the same account and email address. |
|
@dbkr 'to have /different/ pushers for the same account and email address'? i'm lost - perhaps some comments on getEmailPusher and addEmailPusher would help? |
|
Done - is this any clearer? |
|
lgtm |
Supports element-hq/element-web#1469 (this PR is safe without the vector-web PR)
element-hq/element-web#108