Skip to content
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

[$250] Web - Chat - Message marked as new after reading it and scrolling up and down the conversation #50318

Closed
1 of 6 tasks
IuliiaHerets opened this issue Oct 6, 2024 · 14 comments
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 External Added to denote the issue can be worked on by a contributor Help Wanted Apply this label when an issue is open to proposals by contributors

Comments

@IuliiaHerets
Copy link

IuliiaHerets commented Oct 6, 2024

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!


Version Number: v9.0.45-2
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught during regression testing, add the test name, ID and link from TestRail: https://expensify.testrail.io/index.php?/tests/view/5050520
Issue reported by: Applause Internal Team

Action Performed:

  1. Open https://staging.new.expensify.com/
  2. Open the chat with long history.
  3. Scroll up until last messages are not visible.
  4. Send a message.
  5. Verify the chat scrolls to the bottom. The message is marked New.
  6. Scroll up and down a few times, click the "New message" button.

Expected Result:

The "New" mark for the message is dismissed after reading the message and scrolling up and down the conversation.

Actual Result:

The message remains marked as new after reading it and scrolling up and down the chat conversation.

Workaround:

Unknown

Platforms:

  • Android: Native
  • Android: mWeb Chrome
  • iOS: Native
  • iOS: mWeb Safari
  • MacOS: Chrome / Safari
  • MacOS: Desktop

Screenshots/Videos

Bug6626402_1728243616105.New_message.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~021843398922657090244
  • Upwork Job ID: 1843398922657090244
  • Last Price Increase: 2024-10-07
Issue OwnerCurrent Issue Owner: @paultsimura
@IuliiaHerets IuliiaHerets added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Oct 6, 2024
Copy link

melvin-bot bot commented Oct 6, 2024

Triggered auto assignment to @lschurr (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

@IuliiaHerets
Copy link
Author

@lschurr FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors

@lschurr lschurr added the External Added to denote the issue can be worked on by a contributor label Oct 7, 2024
@melvin-bot melvin-bot bot changed the title Web - Chat - Message marked as new after reading it and scrolling up and down the conversation [$250] Web - Chat - Message marked as new after reading it and scrolling up and down the conversation Oct 7, 2024
Copy link

melvin-bot bot commented Oct 7, 2024

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

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Oct 7, 2024
Copy link

melvin-bot bot commented Oct 7, 2024

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

@shijir0927
Copy link

Proposal


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

Message remains marked as new after reading it by clicking on "New messages" floating button and scrolling up and down the chat conversation.

What is the root cause of that problem?

The first bug which is the "New messages" floating button not marking the new message as read was caused by scrollToBottomAndMarkReportAsRead function not fully marking the new report as read as we are missing the second argument when calling Report.readNewestAction(report.reportID);. Without the second argument being true, it doesn't emit readNewestAction_${report_id} event.

Report.readNewestAction(report.reportID);

As for scrolling to the bottom of the message list not marking the message as read, currently, there is no logic tied to handling new messages as read in trackVerticalScrolling function.

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

Explicitly dispatching readNewestAction_${reportID} event with

Report.readNewestAction(report.reportID, true);

fixes our issue for the floating button.

Also, we can add a new logic to mark the unread message as read when there is an unread message and we are scrolled to the bottom of the chat.

    const handleUnreadReportWhenScrolledToTheBottom = () => {
        if (scrollingVerticalOffset.current <= 0 && !!unreadMarkerReportActionID) {
            Report.readNewestAction(report.reportID, true);
        }
    }

    const trackVerticalScrolling = (event: NativeSyntheticEvent<NativeScrollEvent>) => {
        scrollingVerticalOffset.current = event.nativeEvent.contentOffset.y;
        handleUnreadFloatingButton();
        handleUnreadReportWhenScrolledToTheBottom();
        onScroll?.(event);
    };

test branch: shijir0927@6e13ded

fixed state video:

after.mov

What alternative solutions did you explore? (Optional)

With more debugging, we can try to find out what state and where exactly we are ending up in this buggy situation but with bunch of different useEffects happening at the same time in this component making the event cycle pretty complex, I decided to focus on fixing the outcome rather than preventing the code to never reach this state.

@paultsimura
Copy link
Contributor

@MonilBhavsar I see you supervised #42568, which changed this floating pill behavior.

Could you please double-check and confirm: do we want to reset the unread green line marker when clicking the "New messages" floating button?

@MonilBhavsar
Copy link
Contributor

Hey!
We don't want to reset the unread marker when clicking on "New" or scrolling. Referenced from slack behavior.

Also, in video a new marker line is created when user sends the message, which appears like a bug and could be handled in a new report and issue

@paultsimura
Copy link
Contributor

Thanks, so this one is not a bug and can be closed.

cc: @lschurr

@MonilBhavsar
Copy link
Contributor

👍
I just tried but could not reproduce the issue as in video - new marker line appearing after sending a message. Do you please mind checking while we're here

@paultsimura
Copy link
Contributor

@MonilBhavsar I can reproduce, but you have to be quick 😄
From my observation, you have to scroll up immediately after sending the message before the BE response comes. Looks quite edge case-y though:

2024-10-08.-.14.19.-.Screen.Recording.2024-10-08.at.14.18.44.mp4

@MonilBhavsar
Copy link
Contributor

Gotcha, thanks!
it also happens when you send a message when user is already scrolled up reading old messages which seems like usual case. Do you want to report a bug in #expensify-bugs?

@paultsimura
Copy link
Contributor

I will 👌

@paultsimura
Copy link
Contributor

@lschurr / @MonilBhavsar please close this issue.

@MonilBhavsar
Copy link
Contributor

Yes, we created a new issue. So closing this one out. Thanks all!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 External Added to denote the issue can be worked on by a contributor Help Wanted Apply this label when an issue is open to proposals by contributors
Projects
None yet
Development

No branches or pull requests

5 participants