[HOLD for payment 2024-03-14] [HOLD for payment 2024-02-07] [$500] HIGH: Mark the chat as "read" immediately upon switching to the tab #33680

quinthar · 2023-12-28T00:41:29Z

Current behaviour:

User is viewing a chat A and opens new tab.
User receives messages in chat A and comes back to the tab with chat A being open.
Currently the LHN shows as bold, but there is no Unread green line marker.
If the user switches to another chat and comes back to the chat A, now chat A behaves as showing a new unread chat. The LHN bold is gone and unread green marker is shown.

Expected behaviour:

In step 3, LHN shouldn't be bold, though it should have the green unread marker should be shown to symbolize the new messages received.

When the user switches away, the green unread marker should be removed.

Upwork Automation - Do Not Edit

Upwork Job URL: https://www.upwork.com/jobs/~012a54894c108d9212
Upwork Job ID: 1745132109856296960
Last Price Increase: 2024-01-10
Automatic offers:

abdulrahuman5196 | Reviewer | 28093546
FitseTLT | Contributor | 28093547

melvin-bot · 2023-12-28T13:16:38Z

Job added to Upwork: https://www.upwork.com/jobs/~01962a536f7d747895

melvin-bot · 2023-12-28T13:16:40Z

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

melvin-bot · 2023-12-28T13:16:42Z

Bug0 Triage Checklist (Main S/O)

This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
This bug is not a duplicate report (check E/App issues and #expensify-bugs)
- If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
This bug is reproducible using the reproduction steps in the OP. S/O
- If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
- If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
This issue is filled out as thoroughly and clearly as possible
- Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

melvin-bot · 2023-12-28T13:16:44Z

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

rayane-djouah · 2023-12-28T13:40:47Z

Proposal

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

chats are not marked as "read" immediately when a user switches to the chat tab.

What is the root cause of that problem?

the read status of a chat is not updated in real-time as the user views it. Instead, the status update appears to be dependent on a state change triggered by navigating away from and back to the report

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

React Native's AppState can be used to determine if the app is in the foreground or background, and the useEffect hook can be employed to run side effects based on state changes. Here's a proposed solution:

Import AppState from React Native: This module helps to determine the current state of the app.

Add AppState Listener in useEffect: We will add a listener to AppState to detect when the app is brought to the foreground.

Update Read Status: When the app comes to the foreground and the relevant chat tab is active, we trigger the action to mark the chat as read.

import { AppState } from 'react-native';

useEffect(() => {
    const handleAppStateChange = (nextAppState) => {
        // Check if the app is coming to the foreground 
        if (nextAppState === 'active') {
                if (Visibility.isVisible() && Visibility.hasFocus() && ReportUtils.isUnread(report)) {
                    // mark the chat as read
                    Report.readNewestAction(report.reportID);
                }
        }
    };

    AppState.addEventListener('change', handleAppStateChange);

    return () => {
        AppState.removeEventListener('change', handleAppStateChange);
    };
}, [report.reportID]);

What alternative solutions did you explore? (Optional)

tienifr · 2023-12-28T13:42:07Z

Proposal

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

This is new feature, we want to mark chat as read when the chat gains focus.

What is the root cause of that problem?

This is new feature.

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

First we want to listen to the screen visibility in order to run things like reseting marker, mark as read, ...
We can define a new useEffect like below (we can add the Visibility.hasFocus as well if we want to, because even if the app is visible but it doesn't have focus, we might not want to consider it as "read")

const useScreenVisibility = () => {
    const [isVisible, setIsVisible] = useState(Visibility.isVisible());

    useEffect(() => {
        const unsubscribeVisibilityListener = Visibility.onVisibilityChange(() => {
            setIsVisible(Visibility.isVisible());
        });

        return unsubscribeVisibilityListener;
    }, []);

    return isVisible;
}

This can be reused for various use case in the app

We want to read the report right after the user comes back to view the screen, so can add this to here

useEffect(() => {
        if (!isScreenVisible || scrollingVerticalOffset.current >= MSG_VISIBLE_THRESHOLD) {
            return;
        }
                    
        Report.readNewestAction(report.reportID);
        
        if (!currentUnreadMarker) {
            userActiveSince.current = DateUtils.getDBTime();
        }
    }, [report.reportID, isScreenVisible]);

Basically if the user comes back to the screen and the user is viewing the latest messages, then call Report.readNewestAction to read the report. We'll also set userActiveSince.current if the currentUnreadMarker is not present, to acknowledge that the user gets active in the page at that time, this will be used for step 3 below

In here, we should add isScreenVisible to the dependency list so that the marker will be recalculated if the user becomes active in the page again, this will make sure the green marker shows correctly (because the userActiveSince.current needs to be correct for this condition to work, we already set userActiveSince.current to the correct value in step 2)

We can optionally add

if (!isScreenVisible) {
    return;
}

To here because we don't need to recalculate the green marker if the screen is not visible.

Inside readNewestAction we can optionally early return if the report is already read.

Note:

scrollingVerticalOffset.current < MSG_VISIBLE_THRESHOLD (If the user is scrolled out of view, the user didn't see the new message yet so we can't consider it as read, this is the same condition as in here).

What alternative solutions did you explore? (Optional)

We can modify this existing logic to handle the above cases, and we need to make sure to add screen visibility to the dependency list here

tienifr · 2023-12-28T14:24:01Z

Proposal updated to clarify and add example code changes

FitseTLT · 2023-12-28T19:17:22Z

@quinthar I have some suggestions

Alice is viewing the Bob chat in her NewDot tab, but switches to a different tab to do something else. She gets a message from Bob, and switches back to the tab to view it.

Nope. Now unread marker will only display if you open the report while it has unread messages. If you are already on the report and you receive message while you 're on another tab and switch it back it will not display the marker.

Mark chats as read immediately upon becoming visible in the foreground.

Then what is the reason to display the marker if we remove it immediately, in any other social media apps (e.g. telegram) the marker will exist until you open other chat and I think the current behaviour is correct.

bfitzexpensify · 2023-12-28T19:30:10Z

@abdulrahuman5196 this is one marked as critical, so please prioritise reviewing the proposals here - thanks!

quinthar · 2023-12-28T19:45:28Z

Nope. Now unread marker will only display if you open the report while it has unread messages. If you are already on the report and you receive message while you 're on another tab and switch it back it will not display the marker.

Ah, good questions! To clarify, I'm not referring to the "unread marker" (eg, the horizontal line across the chat that designates new from old messages). I'm referring to it being marked as unread (bold) in the LHN. I'm sorry for being ambiguous in the original description, does that clarify things?

FitseTLT · 2023-12-28T20:36:23Z

Proposal

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

Mark the chat as "read" immediately upon switching to the tab

What is the root cause of that problem?

New Feature:
We already have the logic here

App/src/pages/home/report/ReportActionsList.js

Lines 207 to 209 in 88d837c

 if (ReportUtils.isUnread(report)) { 

 if (Visibility.isVisible() && scrollingVerticalOffset.current < MSG_VISIBLE_THRESHOLD) { 

 Report.readNewestAction(report.reportID);

but it only marks the report read if it is Visibility.isVisible so once we return from other tab the useEffect will not be invoked as it only depends on report.lastVisibleActionCreated and report.reportID not visibility state

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

We should add visibility change listener and should Report.readNewestAction when Visibility.isVisible
and the scroll is below the MSG_VISIBLE_THRESHOLD

CAUTION: we should not reset currentUnreadMarker as that will remove the new message green unread marker line which should not be removed until we open another report (we only want the report to be read i.e. remove the boldness in LHN)

So inside ReportActionsList we add this code

useEffect(() => {
    const unsubscribeVisibilityListener = Visibility.onVisibilityChange(() => {

        InteractionManager.runAfterInteractions(() => {
            if (!Visibility.isVisible() || scrollingVerticalOffset.current >= MSG_VISIBLE_THRESHOLD) {
                return;
            }
           if (!currentUnreadMarker) {
                    userActiveSince.current = DateUtils.getDBTime();

                    _.each(sortedReportActions, (reportAction, index) => {
                        if (!shouldDisplayNewMarker(reportAction, index)) {
                            return;
                        }

                        if (!currentUnreadMarker && currentUnreadMarker !== reportAction.reportActionID) {
                            cacheUnreadMarkers.set(report.reportID, reportAction.reportActionID);
                            setCurrentUnreadMarker(reportAction.reportActionID);
                        }
                    });
                }
            Report.readNewestAction(report.reportID);
        });
        
    })

    return unsubscribeVisibilityListener;
}, [report.reportID]);

or We can also use this code

App/src/pages/home/report/ReportActionsList.js

Lines 207 to 209 in 88d837c

 if (ReportUtils.isUnread(report)) { 

 if (Visibility.isVisible() && scrollingVerticalOffset.current < MSG_VISIBLE_THRESHOLD) { 

 Report.readNewestAction(report.reportID);

but we should add visibility state as a dependency here

App/src/pages/home/report/ReportActionsList.js

Line 223 in 88d837c

}, [report.lastVisibleActionCreated, report.reportID]);

What alternative solutions did you explore? (Optional)

bfitzexpensify · 2023-12-29T15:05:51Z

Bump @abdulrahuman5196 - please review these proposals, thank you!

tienifr · 2023-12-29T17:19:19Z

Proposal updated with minor change, based on the requirement update

FitseTLT · 2023-12-29T17:32:21Z

Proposal updated with minor change, based on the requirement update

       // put this if we want to clear the unread green marker as well after coming back, if not then ignore it

@abdulrahuman5196 please take a note that this is not a minor change but the main point of the new feature we are implementing. As only making the report unread and not resetting currentUnreadMarker is the critical and main point; and resetting currentUnreadMarker will make the unread marker useless as it will make it disappear as soon as the user opens the chat (as the visibility will be focused then) although it should be displayed until the user moves to other report or chat

abdulrahuman5196 · 2024-01-01T17:53:04Z

Reviewing now

abdulrahuman5196 · 2024-01-01T18:18:45Z

Current Experience:

Screen.Recording.2024-01-01.at.11.33.07.PM.mov

User is viewing a chat A and opens new tab.
User receives messages in chat A and comes back to the tab with chat A being open.
Currently the LHN shows as bold, but there is no Unread green line marker.
If the user switches to another chat and comes back to the chat A, now chat A behaves as showing a new unread chat. Like the LHN bold is gone and unread green marker is shown.

I think there are 2 issues here regarding the LHN and unread marker here

I think in the step 3, we should change the experience like.
LHN shouldn't be bold, but green marker should be shown to symbolize the new messages received
(which is like a user going to new unread chat)

If we fix step 3, in step 4 the chat would automatically act as already read chat.

Kindly let me if we should change the unread marker as suggested or just have the LHN issue as mentioned in OP.

melvin-bot · 2024-01-01T18:19:34Z

@bfitzexpensify, @abdulrahuman5196 Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

bfitzexpensify · 2024-01-01T20:53:38Z

@abdulrahuman5196 in step 4, when the user switches away, that would remove the green unread marker?

abdulrahuman5196 · 2024-01-02T10:44:24Z

@abdulrahuman5196 in step 4, when the user switches away, that would remove the green unread marker?

That should be the case IMO, but not happening right now. Atleast in this case if we switch to chat B and come back to chat A after step 3, we are seeing unread marker.
@bfitzexpensify

bfitzexpensify · 2024-01-02T13:02:51Z

Cool, yep, I agree that that should be the case. I think the only other thing to mention is that if the user sends a message on chat A, then we should remove the unread marker, too.

FitseTLT · 2024-01-02T21:22:49Z

Updated proposal according to this expectation/comment

bfitzexpensify · 2024-02-07T06:39:56Z

Sweet. Looks like you're up on reviewing the fix @abdulrahuman5196

alexpensify · 2024-02-08T00:10:41Z

Whoops, I missed that @bfitzexpensify is back online:

#33680 (comment)

I'm stepping away here and Ben will deal with the BZ next steps when that time comes.

bfitzexpensify · 2024-02-15T06:24:15Z

Mind taking a look at the PR @abdulrahuman5196?

abdulrahuman5196 · 2024-02-15T07:07:45Z

Hi @bfitzexpensify Sorry for the delay. Will check on it today.

bfitzexpensify · 2024-03-06T05:38:20Z

@abdulrahuman5196 what's the latest with this issue?

abdulrahuman5196 · 2024-03-06T07:07:19Z

@bfitzexpensify new changes have been merged yesterday

hayata-suenaga · 2024-03-06T18:44:35Z

yep the pr was merged

melvin-bot · 2024-03-07T23:44:23Z

Reviewing label has been removed, please complete the "BugZero Checklist".

melvin-bot · 2024-03-07T23:44:26Z

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.4.48-0 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

Fix - Mark the chat as "read" immediately upon switching to the tab #37021

If no regressions arise, payment will be issued on 2024-03-14. 🎊

For reference, here are some details about the assignees on this issue:

@FitseTLT requires payment automatic offer (Contributor)
@abdulrahuman5196 requires payment automatic offer (Reviewer)

melvin-bot · 2024-03-07T23:44:27Z

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

[@abdulrahuman5196] The PR that introduced the bug has been identified. Link to the PR:
[@abdulrahuman5196] 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:
[@abdulrahuman5196] 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:
[@abdulrahuman5196] Determine if we should create a regression test for this bug.
[@abdulrahuman5196] 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.
[@bfitzexpensify] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

bfitzexpensify · 2024-03-15T10:14:21Z

Payments complete, please complete the BZ checklist when you get a moment @abdulrahuman5196 - thank you!

abdulrahuman5196 · 2024-03-19T18:01:49Z

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. New implementation.

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.

Login with User B and open a chat with User A and change to another tab on web
Now Send a message as User A to User B
Wait until the message arrives at User B and now as User B switch back to the tab you were chatting in (1)
Check that the LHN is not bold
verify that unread marker indicates on the new message you sent in (2)

@bfitzexpensify

abdulrahuman5196 · 2024-03-26T18:13:10Z

I think we can close this @bfitzexpensify

slafortune added External Added to denote the issue can be worked on by a contributor Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Dec 28, 2023

melvin-bot bot changed the title ~~CRITICAL: Mark the chat as "read" immediately upon switching to the tab~~ [$500] CRITICAL: Mark the chat as "read" immediately upon switching to the tab Dec 28, 2023

melvin-bot bot assigned bfitzexpensify Dec 28, 2023

melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Dec 28, 2023

melvin-bot bot assigned abdulrahuman5196 Dec 28, 2023

melvin-bot bot added the Overdue label Jan 1, 2024

melvin-bot bot removed the Overdue label Jan 1, 2024

melvin-bot bot removed the Overdue label Feb 5, 2024

alexpensify changed the title ~~[HOLD for payment 2024-02-07] [HOLD for payment 2024-02-05] [$500] CRITICAL: Mark the chat as "read" immediately upon switching to the tab~~ [HOLD for payment 2024-02-07] [$500] CRITICAL: Mark the chat as "read" immediately upon switching to the tab Feb 6, 2024

FitseTLT mentioned this issue Feb 6, 2024

Fix - Mark the chat as "read" immediately upon switching to the tab #35948

Closed

50 tasks

melvin-bot bot added Reviewing Has a PR in review Weekly KSv2 and removed Daily KSv2 labels Feb 6, 2024

FitseTLT mentioned this issue Feb 6, 2024

[HOLD for payment 2024-05-30] [$500] Desktop - Chat isn't marked as read if user clicks on notification from Notification Center #34829

Closed

6 tasks

alexpensify removed their assignment Feb 8, 2024

FitseTLT mentioned this issue Feb 21, 2024

Fix - Mark the chat as "read" immediately upon switching to the tab #37021

Merged

50 tasks

melvin-bot bot added Weekly KSv2 and removed Weekly KSv2 labels Feb 21, 2024

melvin-bot bot added Weekly KSv2 and removed Weekly KSv2 labels Mar 7, 2024

melvin-bot bot changed the title ~~[HOLD for payment 2024-02-07] [$500] CRITICAL: Mark the chat as "read" immediately upon switching to the tab~~ [HOLD for payment 2024-03-14] [HOLD for payment 2024-02-07] [$500] CRITICAL: Mark the chat as "read" immediately upon switching to the tab Mar 7, 2024

melvin-bot bot removed the Reviewing Has a PR in review label Mar 7, 2024

hayata-suenaga closed this as completed Mar 27, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[HOLD for payment 2024-03-14] [HOLD for payment 2024-02-07] [$500] HIGH: Mark the chat as "read" immediately upon switching to the tab #33680

[HOLD for payment 2024-03-14] [HOLD for payment 2024-02-07] [$500] HIGH: Mark the chat as "read" immediately upon switching to the tab #33680

quinthar commented Dec 28, 2023 •

edited by melvin-bot bot

Loading

melvin-bot bot commented Dec 28, 2023

melvin-bot bot commented Dec 28, 2023

melvin-bot bot commented Dec 28, 2023

melvin-bot bot commented Dec 28, 2023

rayane-djouah commented Dec 28, 2023 •

edited

Loading

tienifr commented Dec 28, 2023 •

edited

Loading

tienifr commented Dec 28, 2023

FitseTLT commented Dec 28, 2023 •

edited

Loading

bfitzexpensify commented Dec 28, 2023

quinthar commented Dec 28, 2023

FitseTLT commented Dec 28, 2023 •

edited

Loading

bfitzexpensify commented Dec 29, 2023

tienifr commented Dec 29, 2023

FitseTLT commented Dec 29, 2023

abdulrahuman5196 commented Jan 1, 2024

abdulrahuman5196 commented Jan 1, 2024

melvin-bot bot commented Jan 1, 2024

bfitzexpensify commented Jan 1, 2024

abdulrahuman5196 commented Jan 2, 2024

bfitzexpensify commented Jan 2, 2024

FitseTLT commented Jan 2, 2024

bfitzexpensify commented Feb 7, 2024

alexpensify commented Feb 8, 2024

bfitzexpensify commented Feb 15, 2024

abdulrahuman5196 commented Feb 15, 2024

bfitzexpensify commented Mar 6, 2024

abdulrahuman5196 commented Mar 6, 2024

hayata-suenaga commented Mar 6, 2024

melvin-bot bot commented Mar 7, 2024

melvin-bot bot commented Mar 7, 2024

melvin-bot bot commented Mar 7, 2024

bfitzexpensify commented Mar 15, 2024

abdulrahuman5196 commented Mar 19, 2024

abdulrahuman5196 commented Mar 26, 2024

[HOLD for payment 2024-03-14] [HOLD for payment 2024-02-07] [$500] HIGH: Mark the chat as "read" immediately upon switching to the tab #33680

[HOLD for payment 2024-03-14] [HOLD for payment 2024-02-07] [$500] HIGH: Mark the chat as "read" immediately upon switching to the tab #33680

Comments

quinthar commented Dec 28, 2023 • edited by melvin-bot bot Loading

melvin-bot bot commented Dec 28, 2023

melvin-bot bot commented Dec 28, 2023

melvin-bot bot commented Dec 28, 2023

Bug0 Triage Checklist (Main S/O)

melvin-bot bot commented Dec 28, 2023

rayane-djouah commented Dec 28, 2023 • edited Loading

Proposal

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

What is the root cause of that problem?

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

What alternative solutions did you explore? (Optional)

tienifr commented Dec 28, 2023 • edited Loading

Proposal

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

What is the root cause of that problem?

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

What alternative solutions did you explore? (Optional)

tienifr commented Dec 28, 2023

FitseTLT commented Dec 28, 2023 • edited Loading

bfitzexpensify commented Dec 28, 2023

quinthar commented Dec 28, 2023

FitseTLT commented Dec 28, 2023 • edited Loading

Proposal

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

What is the root cause of that problem?

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

What alternative solutions did you explore? (Optional)

bfitzexpensify commented Dec 29, 2023

tienifr commented Dec 29, 2023

FitseTLT commented Dec 29, 2023

abdulrahuman5196 commented Jan 1, 2024

abdulrahuman5196 commented Jan 1, 2024

melvin-bot bot commented Jan 1, 2024

bfitzexpensify commented Jan 1, 2024

abdulrahuman5196 commented Jan 2, 2024

bfitzexpensify commented Jan 2, 2024

FitseTLT commented Jan 2, 2024

bfitzexpensify commented Feb 7, 2024

alexpensify commented Feb 8, 2024

bfitzexpensify commented Feb 15, 2024

abdulrahuman5196 commented Feb 15, 2024

bfitzexpensify commented Mar 6, 2024

abdulrahuman5196 commented Mar 6, 2024

hayata-suenaga commented Mar 6, 2024

melvin-bot bot commented Mar 7, 2024

melvin-bot bot commented Mar 7, 2024

melvin-bot bot commented Mar 7, 2024

bfitzexpensify commented Mar 15, 2024

abdulrahuman5196 commented Mar 19, 2024

abdulrahuman5196 commented Mar 26, 2024

quinthar commented Dec 28, 2023 •

edited by melvin-bot bot

Loading

rayane-djouah commented Dec 28, 2023 •

edited

Loading

tienifr commented Dec 28, 2023 •

edited

Loading

FitseTLT commented Dec 28, 2023 •

edited

Loading

FitseTLT commented Dec 28, 2023 •

edited

Loading