fix: fix mailserver popup showing after canceling it #10342
Conversation
Fixes #10065 After clicking cancel, the popup is `dismissed`, so it will not appear again Selecting a new mail server sets `dismissed` as false, so the error can appear again
|
Hey @jrainville, and thank you so much for making your first pull request in status-react! ❤️ Please help us make your experience better by filling out this brief questionnaire https://goo.gl/forms/uWqNcVpVz7OIopXg2 |
Pull Request Checklist
|
Jenkins Builds
|
|
@jrainville thanks for the PR! great job! but this issue much deeper than it looks, Mailserver management is completely outdated and we need to reintroduce it with proper design and flows, but even if we want to fix the popup we should fix the cause - adding mailserver process should be stopped, not the consequences - multi popup |
There was a problem hiding this comment.
Nice @jrainville !
Learning a lot from your changes. I've left some inline comments about code-style related things.
src/status_im/mailserver/core.cljs
Outdated
| preferred-mailserver]) | ||
| :style "default"}]}}) | ||
| (update-mailserver-state db :error)} | ||
| (let [error-dismissed (connection-error-dismissed db)] |
There was a problem hiding this comment.
Shall we call this error-dismissed? instead? It seems to be common practice to suffix boolean values with ?. Or does this only apply to certain booleans?
There was a problem hiding this comment.
Thanks for letting me know, I'll change it.
| :advanced-settings | ||
| :offline-messaging-settings)]) | ||
| :extra-options [{:text (i18n/label :t/mailserver-retry) | ||
| :onPress #(re-frame/dispatch |
There was a problem hiding this comment.
This is unrelated to the changes in this PR, but I'm wondering if there's a reason onPress is written in camelCase here while most other events seem to be using kebab-case
There was a problem hiding this comment.
This was code that was already there, I just changed the indentation. I can change it to kebab-case if it's an available prop though.
There was a problem hiding this comment.
yeah, as i said its a really old code, should be re-implemented from scratch
Summary
Fixes #10065 by setting a
dismissedflag on Cancel, so the popup will not appear again.Selecting a new mail server sets
dismissedas false, so the error can appear againPlatforms
Tested Android simulator
Steps to test
See issue above to see reproduction steps