Changed webmail to mail #888
Conversation
src/app/app-settings.ts
Outdated
| @@ -54,14 +54,14 @@ export class AppSettingsService { | |||
| constructor( | |||
| private storage: StorageService, | |||
| ) { | |||
| this.storage.getSubject('webmailSettings').pipe(filter(s => s)).subscribe( | |||
| this.storage.getSubject('mailSettings').pipe(filter(s => s)).subscribe( | |||
There was a problem hiding this comment.
This will effectively reset settings for existing users, who currently have them under webmailSettings. We could migrate them, but I don't think it's worth all the effort in the APIs and internals – I think the original intention was to just change it in the user-facing bits. @gtandersen?
There was a problem hiding this comment.
Yes, "any references" in #886 was meant to include items visible in user interfaces only. :)
There was a problem hiding this comment.
@tadzik @gtandersen I didn't get what this actually mean? Could you please let me know where exactly i have to make correction in PR?
There was a problem hiding this comment.
Just skip the above change -- only human-readable instances of "webmail" should be changed to "mail".
There was a problem hiding this comment.
This still breaks the settings, for example in https://github.com/runbox/runbox7/pull/888/files#diff-97880cd37962b5628664ad1984e174376d6cda35dfefb3dc0376c204516e0998R65.
The change should only concern the words that the user can see – to put it simply, those appear in the HTML files (and instances like https://github.com/runbox/runbox7/pull/888/files#diff-884f7f49640e5923f6bcac4c51d90340330a178f662defbe61e5f5aac1c512deR1130) :) In the internals it's not a problem – it's in fact useful to distinguish between a singular mail (as in, a message), and webmail the application component.
In https://github.com/runbox/runbox7/pull/888/files#diff-ce434ef5cc161194b450135a3586eb89357fc270e8aaa8e336db5a04e8d52fbeR15 both words would actually be helpful – hinting the search engines that we deal with webmail is still useful as this is a term people may use to find us.
This PR will resolve issue #886 .