-
Notifications
You must be signed in to change notification settings - Fork 985
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix error and app freezing when deselecting account one by one #18759
Conversation
Jenkins BuildsClick to see older builds (12)
|
c006739
to
ed8e7c2
Compare
ed8e7c2
to
8666e8e
Compare
src/status_im/config.cljs
Outdated
@@ -167,5 +167,5 @@ | |||
|
|||
(def default-kdf-iterations 3200) | |||
|
|||
(def community-accounts-selection-enabled? (enabled? (get-config :ACCOUNT_SELECTION_ENABLED "0"))) | |||
(def community-accounts-selection-enabled? true) |
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.
can we move this to feature flags now that I merged the UI for feature flags pr? i.e #18602
(not saying it has to be done in this pr)
cc @ilmotta @ajayesivan @FFFra
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.
I believe @ilmotta has done some work related to this. @ilmotta, if you don't have any objections, I can address this in a follow-up PR.
@Parveshdhull, for this PR, I suggest reverting these changes.
44% of end-end tests have passed
Failed tests (26)Click to expandClass TestCommunityMultipleDeviceMerged:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestActivityMultipleDevicePRTwo:
Class TestActivityMultipleDevicePR:
Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestDeepLinksOneDevice:
Expected to fail tests (1)Click to expandClass TestCommunityOneDeviceMerged:
Passed tests (21)Click to expandClass TestCommunityMultipleDeviceMerged:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestActivityMultipleDevicePRTwo:
Class TestActivityMultipleDevicePR:
Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestCommunityOneDeviceMerged:
|
Initially, it was implemented with the local state and later moved to app db as discussed with @ilmotta. |
@Parveshdhull FYI logged a feature request for this here: #18781 |
Hi @qoqobolo, Thank you very much for taking the PR for testing. I think the issue I am mentioning in the PR description is different from #18781. The issue I am reporting is that if we close account selection sheet with swipe (instead of cancel), then new changes will happen, even when we didn't press submit button. (PS, these changes only happens locally, not updated in status-go. So will be reverted once, we reopen community. But until community reopened we have this weird state) |
The fix looks good to me, thanks for your work @Parveshdhull! |
ah well, yes, now I understand what you mean, sorry. |
No. As account selection feature is not enabled in develop, this PR is based on #18530 (As I mentioned in description). So Once the PR is tested and ready, I have to remove those commits from PR before merging.
Thank you 😅 |
8666e8e
to
5592351
Compare
fixes #18753
Summary
Note: PR is based on #18530, and changes will be reverted before merging
What's fixed?
These addresses don’t contain tokens needed to join
Out of Scope
These issues are caused because we are changing the global state while toggling accounts instead of only updating the local state and then submitting when confirm is pressed. Probably there is a reason, why it is implemented this way. (Will log these issues)
status: ready