-
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 "Offline" section in group chat members #19883 #19913
Fix "Offline" section in group chat members #19883 #19913
Conversation
Jenkins BuildsClick to see older builds (8)
|
81% of end-end tests have passed
Failed tests (8)Click to expandClass TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestWalletOneDevice:
Class TestDeepLinksOneDevice:
Class TestWalletMultipleDevice:
Class TestGroupChatMultipleDeviceMergedNewUI:
Expected to fail tests (2)Click to expandClass TestCommunityOneDeviceMerged:
Class TestGroupChatMultipleDeviceMergedNewUI:
Passed tests (42)Click to expandClass TestActivityCenterContactRequestMultipleDevicePR:
Class TestActivityMultipleDevicePRTwo:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestWalletOneDevice:
Class TestCommunityOneDeviceMerged:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Class TestCommunityMultipleDeviceMerged:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestActivityMultipleDevicePR:
|
(re-frame/subscribe [:visibility-status-updates]) | ||
(re-frame/subscribe [:multiaccount/public-key]) | ||
(re-frame/subscribe [:multiaccount/current-user-visibility-status])]) |
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.
should we move this functionality to re-frame/subscribe [:contacts/contacts
itself?
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.
good question, first i wanted to do it, but it should be a separate issue
@Parveshdhull @ibrkhalil thank you for the review, all comments have been addressed |
Could you please edit the PR by adding a detailed description of what exactly was changed here, so that it would be completely clear during testing? |
@qoqobolo done |
Thanks @flexsurfer, it's really helpful 🙏 ISSUE 1: The
|
thannk @qoqobolo fixed |
ISSUE 2:
|
af7ec07
to
e693059
Compare
thanks @qoqobolo fixed |
83% of end-end tests have passed
Failed tests (7)Click to expandClass TestWalletOneDevice:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestDeepLinksOneDevice:
Class TestWalletMultipleDevice:
Expected to fail tests (2)Click to expandClass TestGroupChatMultipleDeviceMergedNewUI:
Class TestCommunityOneDeviceMerged:
Passed tests (43)Click to expandClass TestActivityCenterContactRequestMultipleDevicePR:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestCommunityOneDeviceMerged:
Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Class TestCommunityMultipleDeviceMerged:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestActivityMultipleDevicePR:
Class TestActivityMultipleDevicePRTwo:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestWalletOneDevice:
|
Thanks for your work @flexsurfer, LGTM now! |
fixes #19883
fixes #19895
fixes #19881
Actually offline/online sections have been fixed not removed, cc @qoqobolo