Fix UI freezing when image is opened from activity center #16707
Conversation
Jenkins BuildsClick to see older builds (25)
|
8% of end-end tests have passedFailed tests (33)Click to expandClass TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestActivityMultipleDevicePR:
Class TestCommunityOneDeviceMerged:
Class TestCommunityMultipleDeviceMerged:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestActivityCenterContactRequestMultipleDevicePR:
Passed tests (3)Click to expandClass TestActivityCenterContactRequestMultipleDevicePR:
Class TestCommunityOneDeviceMerged:
|
Parveshdhull
force-pushed
the
fix/ui-freezing-image
branch
from
529612c
to
3d302f9
Compare
|
@Parveshdhull FILE.2023-07-18.11.35.35.mp4The reason why I actually submitted the issue now is that perhaps it could explain/help with other UI freezes which are not easily reproducible. If it is not, the issue itself is not that common and we can definitely postpone it. |
|
Hi @churik, thank you very much for testing the PR.
Yeah sounds good, I will fix the contact request issue and we can include this in the next release.
UI freeze depends on the flow where it happened. If other freezes also happening due to this particular bug, they should be fixed. But we can't know that for sure. There may or may not be other bugs. But whenever a UI freeze issue happens (even randomly), please report it. Even if it is not reproducible, we will know the flow and location of where it happened. |
we would love too, but it seems really random. but ofc we will keep an eye on it! |
|
Apologies, I just saw this PR now. Just sharing a note.
@Parveshdhull - During the implementation of replies, there was no design for all content types. So, the messenger components were used for the time being. Right now, we have the designs for that. Once these designs are implemented, these issues will not occur. The issue to track is #16054 |
|
Hi @smohamedjavid, Thank you very much for information and link. |
Parveshdhull
changed the title
Parveshdhull
force-pushed
the
fix/ui-freezing-image
branch
from
3d302f9
to
a60cc39
Compare
Parveshdhull
changed the title
Parveshdhull
force-pushed
the
fix/ui-freezing-image
branch
from
a60cc39
to
f2c03dd
Compare
|
Hi @smohamedjavid, @ilmotta, |
Parveshdhull
force-pushed
the
fix/ui-freezing-image
branch
from
f2c03dd
to
0efd273
Compare
Parveshdhull
force-pushed
the
fix/ui-freezing-image
branch
from
0efd273
to
8b6987a
Compare
87% of end-end tests have passedFailed tests (5)Click to expandClass TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestCommunityOneDeviceMerged:
Passed tests (34)Click to expandClass TestActivityMultipleDevicePR:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestCommunityOneDeviceMerged:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestCommunityMultipleDeviceMerged:
|
| {:border-radius 12 | ||
| :margin-top 12 | ||
| :padding-horizontal 12 | ||
| :padding-vertical 8 | ||
| :padding-vertical (if (= content-type constants/content-type-image) 12 8) |
There was a problem hiding this comment.
Not sure about this condition's flexibility for other content types.
I think we have only two variants 8 (used on text) and 12 (images, stickers). We can use a boolean to toggle between them depending on the type. Not for this PR. We will need this once we update the designs for other content types.
There was a problem hiding this comment.
Hi @smohamedjavid,
Thank you very much for your input. Updated the condition as per your suggestion. And also instead of passing content-type, passed :attachment prop as per figma.
@ilmotta not sure if there is an issue, but we probably should create Activity logs attachments component and refactor and also simplify activity logs component. wdyt?
There was a problem hiding this comment.
not sure if there is an issue, but we probably should create Activity logs attachments component and refactor and also simplify activity logs component. wdyt?
@Parveshdhull, I totally agree, we need to catch up to the latest changes in the DS and the activity log component is a bit outdated too (based on the guidelines).
There was a problem hiding this comment.
Thank you very much for your input. I updated the PR and removed use of :pointer-events:none. And replaced the use of status-im2.contexts.chat.messages.content.image.view with quo/activity-logs-photos. Please let me know if anything else needs to be changed.
Thanks for the explanation. Great fix @Parveshdhull 🚀
Parveshdhull
force-pushed
the
fix/ui-freezing-image
branch
2 times, most recently
from
aa16ae3
to
522d6f8
Compare
|
@Parveshdhull thanks a lot for the fix! In the discussion above I see that the new component has been added for images in the activity center. But it does not display the image itself sent in the community. Is this expected for now? 
|
|
Hi, @qoqobolo, The issue of missing sent image is also reproducible for me in develop. Please can you check, probably it's different issue. Means, image is visible randomly. Sometimes image disappear after reopening of app or activity centre |
Yeah, I was about to write that the issue of missing images is only reproducible after reopening AC in develop. After this, if you tap on the missing image it leads to UI freezing. So, this issue with frozen UI is not reproducible in this PR. And if I reopen AC or the app, nothing happens, it works just fine. But the image is not displayed the first time AC is opened in PR (while in the develop the image is displayed on the first opening). But 😅 there is another issue with image replies: notification does not appear at all in AC if I send a reply with an image from desktop to mobile (I haven't logged it yet). So speaking of this
If you are sure that it is not this PR that introduces this, then we can log this missing image issue separately. |
Yup, it doesn't look related to PR, as the image is visible if a notification is received while the activity center is open. (So component is working ok) But still, I am not sure about the issue's cause. It might be related to the media server. Probably better to explore it separately. output-2023-07-26_17.51.52.mp4 |
|
@Parveshdhull okay, thanks again for your work and patience! 🙏 |
Parveshdhull
force-pushed
the
fix/ui-freezing-image
branch
from
522d6f8
to
8e8d48f
Compare
fixes: #16706
Summary
We are directly using components created for messaging inside the activity center. And apart from the visible view, those components also have stuff like
on-pressandon-long-pressevents, etc.Not sure if we want to process those events, But If yes we need to properly implement that, we can't navigate to the lightbox and open the image, without opening the chat screen.
status: ready