feat: add option to force encryption#6410
Conversation
r10s
added
blocker
|
See the related PR in Android deltachat/deltachat-android#4431 |
| <option value={Socket.plain}>{tx('off')}</option> | ||
| </DeltaSelect> | ||
|
|
||
| <div className='delta-form-group delta-switch'> |
There was a problem hiding this comment.
Quite misleading placement of the setting, but yep I see that Android's doing the same.
Do we want toggling the switch to take effect immediately or only when the form gets submitted?
There was a problem hiding this comment.
Hmm, good question. The "cancel" button somehow implies "discard my changes" so maybe an unexpected behaviour if we change it immediately. On the other hand this settings is anyway an "expert" feature that should be only changed by people who know what they are doing. And of course "Login" is also a bit strange for saving a value that is not related to the login. But that is all a result of the mentioned "misplacement" of this setting.
There was a problem hiding this comment.
-
if it is not an unreasonable effort, we should apply the changes only when "OK" is hit, we also changed that on android at deltachat/deltachat-android@40faa3d
-
in any case, however, it should be the really last option in the "more settings", so after "certificate checks"
-
for clarity: no need to add it to "untranslated", there is the existing string
enforce_e2eesaying "Enforce Encryption for All Relays", which is clearer. position was discussed a lot, it is fine, if ppl do not find the option without explicitly knowing it exist by other means
There was a problem hiding this comment.
Ok, I changed the behaviour. The setting is not applied when the user cancels the form.
The translation still waits for #6416 to be approved. Then the untranslated entry will be removed.
WofWca
left a comment
WofWca
left a comment
There was a problem hiding this comment.
Tested again, both of the places where LoginForm is used. Works. I didn't test the "error" path though.
The code looks alright. The setting only gets saved on success.
| overflow: visible; | ||
|
|
There was a problem hiding this comment.
| overflow: visible; |
This doesn't do anything, right? It's the default value anyway.
| const [accountSettings, setAccountSettings] = | ||
| useState<Credentials>(defaultCredentials()) | ||
| const [forceEncryption, setForceEncryption] = useState<boolean>( | ||
| settingsStore?.settings['force_encryption'] !== '0' |
There was a problem hiding this comment.
| settingsStore?.settings['force_encryption'] !== '0' | |
| settingsStore?.settings['force_encryption'] === '1' |
We already changed this in one place, so let's be consistent? Especially here where the settingsStore can be nullish. See #6410 (comment)
| [accountId, openDialog, selectAccount, credentials] | ||
| const settingsStore = useSettingsStore()[0] | ||
| const [forceEncryption, setForceEncryption] = useState<boolean>( | ||
| settingsStore?.settings['force_encryption'] !== '0' |
There was a problem hiding this comment.
| settingsStore?.settings['force_encryption'] !== '0' | |
| settingsStore?.settings['force_encryption'] === '1' |
See #6410 (comment)
| SettingsStoreInstance.effect.setCoreSetting( | ||
| 'force_encryption', | ||
| forceEncryption ? '0' : '1' | ||
| ) |
There was a problem hiding this comment.
This is basically unused now, right?
* fix: only save e2ee setting after successful login
3 participants
No description provided.