[$500] CRITICAL: Show Join button in header when hidden

[quinthar]

Problem:
When you open a chat of any kind (eg, room, thread) you generally start out in the hidden notification frequency, meaning that you are just "previewing" the chat -- you haven't joined it, and once you leave it, you won't be notified of any updates. The most straightforward way to join a room is to post. However, you can also explicitly "join" -- and people often want to do so. However, our testing suggests that people have had trouble realizing that they hadn't joined, or that it was needed.

Solution:
Add a Join button to the right side of the header when viewing a room being previewed with with hidden notification frequency. This would look something like this, except with the button merely being Join (because this wouldn't just be shown for threads):

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01c2c22166cbdecd2b
• Upwork Job ID: 1739928998230241280
• Last Price Increase: 2023-12-27
• Automatic offers:
• abdulrahuman5196 | Reviewer | 28072883
• bernhardoj | Contributor | 28072884

[melvin-bot]

Current assignee @srikarparsi is eligible for the Engineering assigner, not assigning anyone new.

[srikarparsi]

Looked into this today and it seems pretty straightforward since we already have the structure setup in the MoneyReportHeader for the button in the header.

We just have to use a regular button with "Join" and add similar logic from MoneyReportHeader in HeaderView.

I'm going to make this external since it's purely frontend but will take it back up if there's no progress made by Friday so that it can be done on Monday.

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~01c2c22166cbdecd2b

[melvin-bot]

Triggered auto assignment to Contributor-plus team member for initial proposal review - @abdulrahuman5196 (External)

[bernhardoj]

Proposal

Please re-state the problem that we are trying to solve in this issue.

There is no Join button on the header of a room/chat with a HIDDEN notification preference.

What is the root cause of that problem?

A new feature request (design change).

What changes do you think we should make in order to solve the problem?

Add the Join button to the left of the 3-dots menu in HeaderView.

App/src/pages/home/HeaderView.js

Lines 286 to 295 in 6365aeb

	<View style={[styles.reportOptions, styles.flexRow, styles.alignItemsCenter]}>
	{isTaskReport && !isSmallScreenWidth && ReportUtils.isOpenTaskReport(props.report) && <TaskHeaderActionButton report={props.report} />}
	{shouldShowThreeDotsButton && (
	<ThreeDotsMenu
	anchorPosition={styles.threeDotsPopoverOffset(windowWidth)}
	menuItems={threeDotMenuItems}
	shouldSetModalVisibility={false}
	/>
	)}
	</View>

We will follow the same logic as the task button to show it at the left of the 3-dots only if it's a large screen. If it's a small screen, we will show it below the header.

We already have a logic to show the Join/Leave button on the 3-dots menu, so we can use the same logic to decide whether to show the Join button separately on the header.

App/src/pages/home/HeaderView.js

Lines 144 to 161 in 6365aeb

	if ((isChatThread && !isEmptyChat) || isUserCreatedPolicyRoom || canLeaveRoom) {
	if (!isWhisperAction && props.report.notificationPreference === CONST.REPORT.NOTIFICATION_PREFERENCE.HIDDEN) {
	threeDotMenuItems.push({
	icon: Expensicons.ChatBubbles,
	text: translate('common.join'),
	onSelected: Session.checkIfActionIsAllowed(() =>
	Report.updateNotificationPreference(props.report.reportID, props.report.notificationPreference, CONST.REPORT.NOTIFICATION_PREFERENCE.ALWAYS, false),
	),
	});
	} else if ((isChatThread && props.report.notificationPreference.length) || isUserCreatedPolicyRoom || canLeaveRoom) {
	const isWorkspaceMemberLeavingWorkspaceRoom = lodashGet(props.report, 'visibility', '') === CONST.REPORT.VISIBILITY.RESTRICTED && isPolicyMember;
	threeDotMenuItems.push({
	icon: Expensicons.ChatBubbles,
	text: translate('common.leave'),
	onSelected: Session.checkIfActionIsAllowed(() => Report.leaveRoom(props.report.reportID, isWorkspaceMemberLeavingWorkspaceRoom)),
	});
	}
	}

[dukenv0307]

Proposal

Please re-state the problem that we are trying to solve in this issue.

Add join button in header for hidden notification room

What is the root cause of that problem?

This is a feature request

What changes do you think we should make in order to solve the problem?

We can use the same way we do for MoneyReportHeader to add a Join Thread button in the right of header if the report has notification as hidden.

App/src/components/MoneyReportHeader.js

Line 146 in 6365aeb

{shouldShowSettlementButton && !isSmallScreenWidth && (

And then when we click on this button we can call Report.updateNotificationPreference function as we do when clicking on Join option of three-dot menu item

What alternative solutions did you explore? (Optional)

NA

[srikarparsi]

Hey @abdulrahuman5196, did you have a chance to look at these proposals?

[abdulrahuman5196]

Hi, yes will look into it today

[abdulrahuman5196]

Checking now

[abdulrahuman5196]

Hi, This seems to be a straightforward feature request. So I think we can go with the first proposal.

But couple of curious questions.

1. What will be the offline case for this? Like would we show the join as greyed or the button being removed
2. Do we have any intimation after pressing the join button, or just the button would go away after pressing? Atleast in other places working now, we would add intimation message to the chat, but that might not be the case here as it seems.

@srikarparsi

[quinthar]

If the current Join button in the overflow works offline, then this should work offline too. If it doesn't, don't worry about it.

[quinthar]

Given that, can you give an ETA for completion?

[quinthar]

Given that, can you give an ETA for completion?

[bernhardoj]

I think we can complete it today

cc: @abdulrahuman5196

[abdulrahuman5196]

If the current Join button in the overflow works offline, then this should work offline too. If it doesn't, don't worry about it.

Got it. Thank you for the information.

[abdulrahuman5196]

Since this is a feature request, going by with the first comment with considerable information.

@bernhardoj 's proposal here #33577 (comment) looks good and works well.

🎀 👀 🎀
C+ Reviewed

cc: @srikarparsi

[melvin-bot]

Triggered auto assignment to @marcochavezf, see https://stackoverflow.com/c/expensify/questions/7972 for more details.

[bernhardoj]

@srikarparsi Should I work on this now?

[srikarparsi]

Yes please, that would be great

[bernhardoj]

PR is ready

cc: @abdulrahuman5196

@srikarparsi I have a question here

[quinthar]

What is this waiting on? Who is this waiting on? I see the above PR has been merged, but I don't see the button in the app. @srikarparsi @bernhardoj -- what's up? What's the ETA can we get this fixed -- can we fix it this week?

[bernhardoj]

but I don't see the button in the app.

May I know which report you open that you don't see the button?

If it's a thread, the leave/join button will show if the thread has at least one message (other than the thread parent).

If it's a public room, from my testing, you will be automatically joined when you open the room.

[quinthar]

Why does it only show in a thread if there is at least one chat? Why wouldn't it always show to anyone who's notification status is hidden?

…

On Sun, Jan 7, 2024, 8:03 PM Bernhard Owen Josephus < ***@***.***> wrote: but I don't see the button in the app. May I know which report you open that you don't see the button? If it's a thread, the leave/join button will show if the thread has at least one message (other than the thread parent). If it's a public room, from my testing, you will be automatically joined when you open the room. — Reply to this email directly, view it on GitHub <#33577 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAEMNUXH24QILEHWENQOH7LYNNVZBAVCNFSM6AAAAABBCW3NGKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQOBQGM2TMMBUG4> . You are receiving this because you authored the thread.Message ID: ***@***.***>

[bernhardoj]

I think that behavior was introduced in this PR which was authored by @srikarparsi. I can't access the internal issue, so I don't know why we do that.

[srikarparsi]

Why does it only show in a thread if there is at least one chat? Why wouldn't it always show to anyone who's notification status is hidden?

We're fixing that behavior in this issue. Here's an explanation for why I originally thought we shouldn't be able to join/leave non-existent threads but I also agree now that people should be allowed to "join" threads even if they haven't been created yet.

[quinthar]

A PR is in review for the remaining issue.

[quinthar]

What's the latest on this? What is preventing us from closing this GH right now?

[srikarparsi]

We can close this out! There's one edge case bug here but we can fix that in that issue. Should be done by this week at the latest.

[bernhardoj]

@srikarparsi I think we are still waiting for the payment.

[abdulrahuman5196]

The PR that introduced the bug has been identified. Link to the PR:
The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:

Not a regression. Feature request.

Determine if we should create a regression test for this bug.

Yes.

If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.

1. Open a policy room chat
2. Send any message
3. In the message context menu, press "Unsubscribe from the thread" (you are automatically subscribed to your own message)
4. In the message context menu, press "Reply in thread"
5. In the thread chat header, verify there is a "Join" button (on web/desktop, the button is to the left of 3-dot, on mobile devices, the button is below the header title and avatar)
6. Press the "Join" button and verify it disappears
7. Press the 3-dot button and choose "Leave". You will be redirected to parent chat.

@srikarparsi Melvin seems to have not updated this bug. This issue should be up for payment now. Kindly add a BZ member regarding the same.

[srikarparsi]

Ah sorry! Reopening and adding the BZ label for payment.

[melvin-bot]

Triggered auto assignment to @MitchExpensify (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

[srikarparsi]

Hey @MitchExpensify! Do you think you could help with paying @abdulrahuman5196 and @bernhardoj for this PR whenever you have a chance?

[MitchExpensify]

Payment summary:

• C: @bernhardoj $500 (Upwork)
• C+: @abdulrahuman5196 $500 (Upwork)

Offers sent! https://www.upwork.com/jobs/~01a8693a9be92d03bf

[melvin-bot]

@MitchExpensify, @abdulrahuman5196, @srikarparsi, @bernhardoj Eep! 4 days overdue now. Issues have feelings too...

[abdulrahuman5196]

@MitchExpensify Could you kindly process payment for this issue?

[MitchExpensify]

Done! Sorry for the delay