Skip to content
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

Merged
merged 2 commits into from
Apr 26, 2023
Merged

Conversation

Parveshdhull
Copy link
Member

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

@status-im-auto
Copy link
Member

status-im-auto commented Apr 24, 2023

Jenkins Builds

Click to see older builds (18)
Commit #️⃣ Finished (UTC) Duration Platform Result
✔️ b9a1f2c #1 2023-04-24 10:24:21 ~6 min tests 📄log
✔️ b9a1f2c #1 2023-04-24 10:24:34 ~6 min android-e2e 🤖apk 📲
✔️ b9a1f2c #1 2023-04-24 10:24:35 ~6 min android 🤖apk 📲
✔️ b9a1f2c #1 2023-04-24 10:24:51 ~6 min ios 📱ipa 📲
d726496 #2 2023-04-25 12:04:07 ~4 min ios 📄log
✔️ d726496 #2 2023-04-25 12:07:14 ~7 min android-e2e 🤖apk 📲
✔️ d726496 #2 2023-04-25 12:08:21 ~8 min android 🤖apk 📲
✔️ d726496 #2 2023-04-25 12:10:37 ~10 min tests 📄log
d726496 #3 2023-04-25 13:16:21 ~2 min ios 📄log
fe9d547 #4 2023-04-25 13:38:16 ~1 min ios 📄log
✔️ fe9d547 #3 2023-04-25 13:42:40 ~6 min android 🤖apk 📲
✔️ fe9d547 #3 2023-04-25 13:43:38 ~7 min android-e2e 🤖apk 📲
✔️ fe9d547 #3 2023-04-25 13:43:40 ~7 min tests 📄log
✔️ fe9d547 #5 2023-04-25 14:00:51 ~7 min ios 📱ipa 📲
✔️ 33631d5 #4 2023-04-25 18:20:38 ~5 min android 🤖apk 📲
✔️ 33631d5 #4 2023-04-25 18:20:46 ~5 min android-e2e 🤖apk 📲
✔️ 33631d5 #4 2023-04-25 18:21:06 ~5 min tests 📄log
✔️ 33631d5 #6 2023-04-25 18:21:30 ~6 min ios 📱ipa 📲
Commit #️⃣ Finished (UTC) Duration Platform Result
✔️ d8b4009 #5 2023-04-26 10:43:08 ~5 min tests 📄log
✔️ d8b4009 #7 2023-04-26 10:43:22 ~6 min ios 📱ipa 📲
✔️ d8b4009 #5 2023-04-26 10:43:25 ~6 min android 🤖apk 📲
✔️ d8b4009 #5 2023-04-26 10:43:33 ~6 min android-e2e 🤖apk 📲
✔️ 6d556dc #6 2023-04-26 10:52:05 ~5 min tests 📄log
✔️ 6d556dc #6 2023-04-26 10:52:35 ~6 min android 🤖apk 📲
✔️ 6d556dc #6 2023-04-26 10:52:37 ~6 min android-e2e 🤖apk 📲
✔️ 6d556dc #8 2023-04-26 10:54:29 ~8 min ios 📱ipa 📲

@qoqobolo qoqobolo self-assigned this Apr 24, 2023
@Parveshdhull Parveshdhull force-pushed the fix/theme-change-bug branch 2 times, most recently from d726496 to fe9d547 Compare April 25, 2023 13:36
@qoqobolo
Copy link
Contributor

Hey @Parveshdhull, thanks for the fixes!
The issue with the background is indeed fixed, however, there is a new related bug:

ISSUE 1: UI freezes if you open the 'chat key' popover and change device theme

Steps:

  1. Open the 'chat key' popover
  2. Change the device theme
  3. Tap your name again

After this step, UI freezes until relogin.

IMG_1431.MP4

@Parveshdhull
Copy link
Member Author

hi @qoqobolo, Thank you very much for testing the PR.
Issue 1 should be fixed now

@qoqobolo
Copy link
Contributor

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:

  1. Open profile
  2. Change the device theme
  3. Close profile

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.mp4

At 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.
Also, after that, you can click on any button on the community or chat screen (i.e. Discover or tabs Joined, Pending, etc.), after which the BROWSER will open, and after that UI starts working normally again.

Screenshot 2023-04-26 at 11 56 28

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 ?
Should I log it separately?

@qoqobolo
Copy link
Contributor

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.
Sorry for the fake ping!

@Parveshdhull
Copy link
Member Author

Hi, thank you for finding this issue, yes it is probably same as #15310, I will look into that next.

@Parveshdhull
Copy link
Member Author

So, I think what happens is on UI reloading, opacity and touch events of different tabs gets activated for some reason

[{:keys [db]}]
{:dissmiss-all-overlays-fx nil
:db (-> db
(dissoc :popover/popover)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what about bottom sheet ?

Copy link
Member Author

@Parveshdhull Parveshdhull Apr 26, 2023

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)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added now

@Parveshdhull Parveshdhull merged commit 0649c66 into develop Apr 26, 2023
@Parveshdhull Parveshdhull deleted the fix/theme-change-bug branch April 26, 2023 11:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Archived in project
7 participants