[15128] Introduce muting for a specific duration #15253
Merged
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
Jenkins BuildsClick to see older builds (134)
|
ibrkhalil
requested review from
flexsurfer,
ilmotta,
rasom and
Parveshdhull
and removed request for
flexsurfer
ibrkhalil
commented
Mar 8, 2023
| [{:keys [db]} chat-id muted?] | ||
| (let [method (if muted? "wakuext_muteChat" "wakuext_unmuteChat")] | ||
| [{:keys [db]} chat-id muted? mute-type] | ||
| (let [method (if muted? "wakuext_muteChatV2" "wakuext_unmuteChat") |
There was a problem hiding this comment.
Gonna have to go with a V2 naming to not break desktop muting functionality
rasom
approved these changes
Mar 8, 2023
ilmotta
reviewed
Mar 8, 2023
ilmotta
approved these changes
Mar 8, 2023
ibrkhalil
force-pushed
the
mute-for-specific-amount-of-time
branch
from
c2a4851 to
5f4bbb7
Compare
|
Hey @ibrkhalil ! Could you please rebase the PR and resolve conflicts? If it is ready for testing feel free to move it to E2E column. Thanx. |
ibrkhalil
force-pushed
the
mute-for-specific-amount-of-time
branch
from
003d67c to
d8cb727
Compare
0% of end-end tests have passedNot executed tests (22)Failed tests (7)Click to expandClass TestGroupChatMultipleDeviceMergedNewUI:
Class TestActivityMultipleDevicePR:
|
ibrkhalil
force-pushed
the
mute-for-specific-amount-of-time
branch
2 times, most recently
from
38ce888 to
f6ec489
Compare
40% of end-end tests have passedNot executed tests (4)Failed tests (15)Click to expandClass TestGroupChatMultipleDeviceMergedNewUI:
Class TestCommunityMultipleDeviceMerged:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestActivityMultipleDevicePR:
Passed tests (10)Click to expandClass TestGroupChatMultipleDeviceMergedNewUI:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestActivityMultipleDevicePR:
|
44% of end-end tests have passedNot executed tests (4)Failed tests (14)Click to expandClass TestCommunityMultipleDeviceMerged:
Class TestActivityMultipleDevicePR:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Passed tests (11)Click to expandClass TestGroupChatMultipleDeviceMergedNewUI:
Class TestCommunityMultipleDeviceMerged:
Class TestActivityMultipleDevicePR:
|
86% of end-end tests have passedFailed tests (4)Click to expandClass TestCommunityMultipleDeviceMerged:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestActivityCenterContactRequestMultipleDevicePR:
Passed tests (25)Click to expandClass TestActivityMultipleDevicePR:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestCommunityMultipleDeviceMerged:
Class TestCommunityOneDeviceMerged:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestActivityCenterContactRequestMultipleDevicePR:
|
|
Hi @ibrkhalil thank you for your work. For now, push notifications are not received when chat is muted. Here are found issues: This issue is created corresponding to Acceptance Criteria described in #14556 (comment) and #15161 (comment) ISSUE 1: The chat unread indicators on chat, AC, and chat icon appear if chat is muted and any message is received on 1-1 or on group chatSteps to reproduce:User_A:
User_B: Actual result:mute_me.mp4
Expected result:When a chat is muted, all notifications should stop e.g. notification bubbles don't appear, |
ISSUE 2: The mentions and replies go to the AC if the chat is mutedSteps to reproduce:User_A:
User_B: Actual result:
Expected result:Replies and mentions sent in the chat while it is muted do not go to AC |
|
hi @pedro-et a few questions related to chat muting Question 2: Should the design be applied for 1-1 chat and for group chat when they are muted as it is implemented for community channels? (maybe this design already exists, but I didn't find it) |
ibrkhalil
force-pushed
the
mute-for-specific-amount-of-time
branch
from
9955f88 to
1c8db22
Compare
|
Hi @ibrkhalil. Thank you fixes. Issues 1,2,3 are fixed. One more issue is found ISSUE 4: The unread badge and unread counter are shown after chat is unmutedSteps to reproduce:
Actual result:
muted_chat.mp4Expected result:All messages received during chat is muted should not be shown as unread on the chat preview after chat is unmuted |
Yes; it's true. A preview of the message should be visible even in muted state.
Here you can see how muted messages should be rendered in the list of messages. https://www.figma.com/file/7KIYbhoqNGAIFonE0w9TDz/Messages-for-Mobile?node-id=1424%3A342207&t=bEzrXQHjVhG2hDX0-1 |
Shouldn't we bring back the indicator on unmute?, I think that's how Telegram and WhatsApp work and personally I think it's nicer to let the user know he missed X amount of messages. |
|
I agree! We didn't design it specifically, but what you are suggesting makes sense to me. |
|
@xAlisher when the chat is muted, should an unread indicator and an unread counter appear just for the chat preview, or also it should cause appearing of old unread messages in the activity center as well? As for me, it is better not to show them in AC (for not to annoy the user). And now it is implemented so. So, if the muted chat is unmuted, then the unread indicator or unread counter will appear in the chat preview and in the chat tab icon (on the screen) if some unread messages were sent when the chat was muted. |
79% of end-end tests have passedFailed tests (6)Click to expandClass TestActivityCenterContactRequestMultipleDevicePR:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestCommunityMultipleDeviceMerged:
Class TestCommunityOneDeviceMerged:
Passed tests (23)Click to expandClass TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestCommunityMultipleDeviceMerged:
Class TestActivityMultipleDevicePR:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestCommunityOneDeviceMerged:
Class TestActivityCenterContactRequestMultipleDevicePR:
|
After unmuting badge appears only on the list. (Not notifying in a bottom bar, but not marked as read) |
In the screenshot, the bottom bar doesn't show a counter of unread messages for chats who are never muted. |
90% of end-end tests have passedFailed tests (3)Click to expandClass TestCommunityMultipleDeviceMerged:
Class TestActivityCenterContactRequestMultipleDevicePR:
Passed tests (26)Click to expandClass TestGroupChatMultipleDeviceMergedNewUI:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestCommunityMultipleDeviceMerged:
Class TestActivityMultipleDevicePR:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestCommunityOneDeviceMerged:
|
|
Hi @ibrkhalil Thank you for your work. PR is ready to be merged |
Hey @ibrkhalil ! Could you please elaborate, what is current muting time for chats (1-1, group chat) in develop? I suppose it is till unmute? 
|
I thought we were going to do the same for chats I added it here actually |
1 similar comment
I thought we were going to do the same for chats I added it here actually |
Oh, cool, thank you. I didn't notice it because I thought that this PR is related to channels only. |
It is, But I though it wouldn't make sense to have a feature in one part and to have it missing in a really similar part. |
If the user has not been in this muted chat and has not seen the messages in the viewing window, an icon should appear after unmuting. |
Design team answer |
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 #15128
Summary
This PR Introduces muting a chat for a specific amount of time
QA notes: Kindly try to mute a chat from the home screen, It should be muted for one minute and then unmuted automatically .. We won't allow users to mute for a minute ofc but this is just to help QA team in testing
This is a PR to introduce the functionality that'll be used here
status: ready