-
Notifications
You must be signed in to change notification settings - Fork 984
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 device theme change listener in ios #15724
Conversation
Jenkins BuildsClick to see older builds (18)
|
d726496
to
fe9d547
Compare
Hey @Parveshdhull, thanks for the fixes! ISSUE 1: UI freezes if you open the 'chat key' popover and change device themeSteps:
After this step, UI freezes until relogin. IMG_1431.MP4 |
fe9d547
to
33631d5
Compare
hi @qoqobolo, Thank you very much for testing the PR. |
Thank you @Parveshdhull! We have a bit weird situation now. In fact, Issue 1 is fixed, but I can still reproduce the freezing UI with the following steps:
Sometimes you need to change the theme a couple of times to reproduce this. And the behavior gets really weird, see the video: video_2023-04-26_11-46-57.mp4At first, UI is completely frozen except for the bottom tabs, but if you click on the wallet tab and then go back to the home screen and tap on the profile many times, the keyboard will open. And the thing is that now it is also reproduced on nightly and both on Android and iOS. Yesterday, for the first time, I ran into a randomly freezing UI in this PR. I did not find the exact steps to reproduce, but I notified @flexsurfer, and it was decided to investigate this bug later. Now these two bugs have overlapped each other, and I'm not sure what we should do in this situation. What do you guys think @Parveshdhull @flexsurfer ? |
UPD: it turned out there is a known issue #15310 and apparently it has nothing to do with the current PR. I'll add steps to reproduce to the issue and will check this PR briefly again, it looks good so far, I just spent some time investigating this mess. |
Hi, thank you for finding this issue, yes it is probably same as #15310, I will look into that next. |
So, I think what happens is on UI reloading, opacity and touch events of different tabs gets activated for some reason |
33631d5
to
d8b4009
Compare
[{:keys [db]}] | ||
{:dissmiss-all-overlays-fx nil | ||
:db (-> db | ||
(dissoc :popover/popover) |
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.
what about bottom sheet ?
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.
hi @flexsurfer, Thank you for the reviewing PR.
I just skipped that one, because for all overlays (navigation/dissmiss-all-overlays)
works/enough, except for above two. For these two we also have to remove them from db. (not sure why)
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.
added now
fixes #15708
fixes #15690
Testing
Please check device theme changes in ios and Android, in foreground and background, and with and without overlay/popover
Note: On theme changing, the app state will remain the same, except for overlays/popovers, they will be dismissed
status: ready