Implement navigation to profiles and chats from contact requests inside Activity Center #19902
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@seanstrom Thank you for creating issue #19649, I have implemented navigation as per requirements mentioned in issue. I have two questions
|
Jenkins BuildsClick to see older builds (19)
|
|
@status-im/mobile-qa Please let me know if you have any opinion on this |
flexsurfer
reviewed
May 6, 2024
| :full-name (profile.utils/displayed-name profile) | ||
| :profile-picture (profile.utils/photo profile)}])) | ||
| [rn/view | ||
| {:on-start-should-set-responder |
There was a problem hiding this comment.
could you pls elaboeate on how on-start-should-set-responder works here? thanks
There was a problem hiding this comment.
hi @flexsurfer, Thank you very much for reviewing PR.
We have the whole activity log as pressable for accepted requests, but when context-tag is pressed we only want that to call an on-press event and avoid propagation to parent pressable.
I picked this solution from the stackoverflow answer, apparently adding onStartShouldSetResponder and returning true is enough for our situation to make it work.
Thanks for the questions 🙏
Side note:
|
Parveshdhull
force-pushed
the
chore/hide-private-channel
branch
from
ad074b9 to
d2b3e41
Compare
|
Thank you @seanstrom for quick response
Please can you elaborate. We do have a on-press handler for accepted contact requests that navigates to chat screen. Do you mean we add this on-press entry for all activity logs? |
Parveshdhull
force-pushed
the
chore/hide-private-channel
branch
from
d2b3e41 to
3a71932
Compare
confirmed with the design team, Lets always navigate to contact profile for consistency. |
|
@flexsurfer @seanstrom Thank you for reviewing PR, Please let me know if anything else needs to be addressed. |
Parveshdhull
force-pushed
the
chore/hide-private-channel
branch
from
3a71932 to
2f7a95d
Compare
seanstrom
approved these changes
May 7, 2024
54% of end-end tests have passedFailed tests (23)Click to expandClass TestCommunityMultipleDeviceMerged:
Class TestDeepLinksOneDevice:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestWalletOneDevice:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestWalletMultipleDevice:
Expected to fail tests (1)Click to expandClass TestCommunityOneDeviceMerged:
Passed tests (28)Click to expandClass TestCommunityMultipleDeviceMerged:
Class TestCommunityOneDeviceMerged:
Class TestActivityMultipleDevicePRTwo:
Class TestActivityMultipleDevicePR:
Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestActivityCenterContactRequestMultipleDevicePR:
|
|
hi @Parveshdhull could you please resolve existing conflicts? Thanx |
|
Hi @Parveshdhull, I have a question. Should the 'contact icon' be interactive? Currently, tapping the 'contact icon' for accepted contacts navigates to a 1-1 chat, while it does not navigate anywhere if tapped for pending or declined contact requests. |
Parveshdhull
force-pushed
the
chore/hide-private-channel
branch
from
2f7a95d to
c06249d
Compare
|
Thank you very much @VolodLytvynenko for picking the PR for testing
done
I am not sure. We probably should discuss this with the design team. But to me, these icons don't look like pressable buttons. They just look like simple indicators to highlight notification types (contact request-related notifications).
When we have accepted contact requests we are making the whole activity log pressable (which includes the contact icon). I am not sure why this was implemented this way and if this is what we wanted. Sorry probably the issue title is a little misleading, but I think for the scope of this issue/PR we can only focus on navigation from the activity center using contact tags. For other components, we currently don't have guidelines and expected behavior. |
85% of end-end tests have passedFailed tests (6)Click to expandClass TestWalletOneDevice:
Class TestWalletMultipleDevice:
Class TestDeepLinksOneDevice:
Class TestGroupChatMultipleDeviceMergedNewUI:
Expected to fail tests (2)Click to expandClass TestCommunityOneDeviceMerged:
Class TestGroupChatMultipleDeviceMergedNewUI:
Passed tests (44)Click to expandClass TestActivityCenterContactRequestMultipleDevicePR:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestCommunityMultipleDeviceMerged:
Class TestWalletOneDevice:
Class TestCommunityOneDeviceMerged:
Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Class TestActivityMultipleDevicePRTwo:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestActivityMultipleDevicePR:
|
|
Hi @Parveshdhull thank you for PR and for clarification. No issues from my side. PR is ready to be merged |
Parveshdhull
force-pushed
the
chore/hide-private-channel
branch
from
c06249d to
e21e9d0
Compare
…de Activity Center
Parveshdhull
force-pushed
the
chore/hide-private-channel
branch
from
e21e9d0 to
9f16898
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
fixes #19649
Testing Note:
Please test navigation behavior as per requirements mentioned in #19649
status: ready