Expensify / App

Welcome to New Expensify: a complete re-imagination of financial collaboration, centered around chat. Help us build the next generation of Expensify by sharing feedback and contributing to the code.
https://new.expensify.com
MIT License
3.52k stars 2.87k forks source link

[$250] Web - Messages - Unread marker is not displayed after the messages are received #44007

Open lanitochka17 opened 4 months ago

lanitochka17 commented 4 months ago

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: 1.4.85-0 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/4632530 Email or phone of affected tester (no customers): test.iris.santos+17062024@gmail.com Issue reported by: Applause - Internal Team

Action Performed:

Preconditions:

Expected Result:

The unread marker (green line and 'New' label) is displayed in the conversation after the messages are received

Actual Result:

The unread marker (green line and 'New' label) is NOT displayed in the conversation after the messages are received

Workaround:

Unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

Screenshots/Videos

Add any screenshot/video evidence

https://github.com/Expensify/App/assets/78819774/5034a743-743b-4579-be4e-d53be27a2a04

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01fbab0fd3569bf04f
  • Upwork Job ID: 1823480826689774019
  • Last Price Increase: 2024-09-03
  • Automatic offers:
    • brunovjk | Reviewer | 103816429
melvin-bot[bot] commented 4 months ago

Triggered auto assignment to @alexpensify (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.

alexpensify commented 4 months ago

I'll review this one soon.

alexpensify commented 4 months ago

Still on my testing radar

alexpensify commented 4 months ago

Other GHs and customer tasks have been a higher priority, I'll get to this one soon.

alexpensify commented 4 months ago

After reviewing these steps, I don't think it's a dupe but there is a known issue with the new message banner when online. It's being addressed here: https://github.com/Expensify/App/issues/43486. I'm going to put this GH on hold and then test it again after #43486 in Production. Setting a reminder to check next week.


Heads up, I will be offline until Monday, July 8, 2024, and will not actively watch over this GitHub during that period.

If this GitHub needs an urgent update, please ask for help in the #expensify-open-source Slack Room. If it can wait, I'll continue the review process when I return online. Thanks!

alexpensify commented 3 months ago

Still on hold for https://github.com/Expensify/App/issues/43486

alexpensify commented 3 months ago

No update, on hold for https://github.com/Expensify/App/issues/43486

alexpensify commented 3 months ago

@bernhardoj and @shubham1206agra - I believe this one is resolved by the other GH - https://github.com/Expensify/App/issues/43486, but let me know if you disagree. Thanks!

bernhardoj commented 3 months ago

The PR for https://github.com/Expensify/App/issues/43486 is actually reverted and the new PR hasn't been merged yet.

alexpensify commented 3 months ago

Thanks, I missed that there was a revert. Ok, keeping this one on hold.

alexpensify commented 3 months ago

Weekly Update: On hold for https://github.com/Expensify/App/issues/43486

alexpensify commented 2 months ago

@bernhardoj and @shubham1206agra can I get your feedback if we should close this one or move forward with the next steps? Thanks!

shubham1206agra commented 2 months ago

@alexpensify Can you please retest this?

trjExpensify commented 2 months ago

Chat bug, removing from wave-collect.

alexpensify commented 2 months ago

SO related to this Process: https://stackoverflowteams.com/c/expensify/questions/14418/14456#14456

@lanitochka17 - can you please retest this one? I added the label too. Thanks!

MelvinBot commented 2 months ago

This has been labelled "Needs Reproduction". Follow the steps here: https://stackoverflowteams.com/c/expensify/questions/16989

kavimuru commented 2 months ago

Bug is still reproducible.

https://github.com/user-attachments/assets/e9ef1309-c100-4f71-a45f-626b42158d66

melvin-bot[bot] commented 2 months ago

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

melvin-bot[bot] commented 2 months ago

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

alexpensify commented 2 months ago

Thanks, I'm marking this one as external, and we are open for proposals here.

alexpensify commented 2 months ago

Waiting for proposals

brunovjk commented 2 months ago

Bumped on Slack for proposals

chrispader commented 2 months ago

Hey i'm Chris from Margelo, an expert agency. I'm investigating the issue right now.

melvin-bot[bot] commented 2 months ago

๐Ÿ“ฃ It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? ๐Ÿ’ธ

chrispader commented 2 months ago

I just wanted to confirm, that the "unread" marker and "New" label are actually supposed to be shown when the user receives messages while still in the chat.

The marker should only be visible when the receiving device was offline, correct? Not when the only sending device was offline...?

Also i'm curious about differences in the provided repro videos:

[CASE 1]: In the video in the issue description, the WiFi is turned off, but only the second (right) tab goes offline. Why is that?

When WiFi is turned on again, the "New messages" popup at the top of the chat appears, but no "unread" marker.

[CASE 2]: In this repro video, only the internet connection of the receiving device is turned off. When the tab goes back online again, there is NO "New messages" popup.

Therefore, what is the expected behavior? Whenever the (receiving) user goes offline and comes back online, there should be both...

... or nothing at all? When the user is online, there should be no indication of unread messages, right?

cc @alexpensify @brunovjk

chrispader commented 2 months ago

I cannot consistently reproduce [CASE 1], i only see the "New messages" banner sometimes.

[CASE 2], where no popup and no "unread" marker appears is reproducible.

Here are some of my screen recordings with different scenarios on both DEV and STG. (both on Google Drive, because they are too big to upload on GH)

DEV: VIDEO STG: VIDEO

alexpensify commented 2 months ago

@brunovjk - when you get a chance, can you please review the questions? Thanks!

brunovjk commented 2 months ago

Sure, I'll get back here ASAP.

brunovjk commented 2 months ago

Apologies for the delay. I'll reach out to an internal to help confirm the expected behavior.

๐ŸŽ€๐Ÿ‘€๐ŸŽ€

melvin-bot[bot] commented 2 months ago

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

brunovjk commented 2 months ago

@tylerkaraszewski, could you help us confirm the expected behavior in this scenario? There's some confusion regarding the "New messages" popup and "unread" markers. Please take a look at @chrispader's comments for more context. Thank you :D

alexpensify commented 2 months ago

Next Steps

We need an internal review of this proposal

Heads up, I will be offline until Tuesday, September 3, 2024, and will not actively watch over this GitHub during that period.

If anything urgent is needed here, please ask for help in the #expensify-open-source Slack Room-- thanks!

melvin-bot[bot] commented 2 months ago

@tylerkaraszewski, @alexpensify, @brunovjk Whoops! This issue is 2 days overdue. Let's get this updated quick!

brunovjk commented 2 months ago

Excuse me @tylerkaraszewski, did you have a chance to look at chrispader's comments? Thanks.

tylerkaraszewski commented 2 months ago

@roryabraham - I picked you here because I feel like you'd know the answer.

I don't actually know what we expect the behavior here to be when you've been offline but come back online while the new messages are in the focused foreground window. I feel like we should not see a notification badge on the conversation, because the conversation is already loaded, but we should see the unread marker line, as this can mean many messages are delivered at once, and that line is used to indicate where you'd been in the conversation previously.

This is fairly nuanced though so I'd love a second opinion.

roryabraham commented 2 months ago

I'd agree with that @tylerkaraszewski, but TBC I'm not sure that this is a "bug" per-se, but a new feature.

Currently, the Unread Indicator only appears when you open a chat after having a different one open. But I agree that it makes sense to also show it if you've been offline and come back online, even if you've had the chat open the whole time ๐Ÿ‘๐Ÿผ

bug v.s: new feature doesn't really matter though in terms of how we address this issue. I think we can move forward with selecting a solution and implementing the new behavior. If we don't already have a regression test covering this case, we should be sure to add one.

melvin-bot[bot] commented 2 months ago

๐Ÿ“ฃ It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? ๐Ÿ’ธ

chrispader commented 2 months ago

Alright, i'm going to set up a PR for this later today, if that's alright?

Going for this approach:

brunovjk commented 2 months ago

Alright, i'm going to set up a PR for this later today, if that's alright?

Going for this approach:

  • Only show a notification badge when the user is not currently in the report
  • Show both "New messages" popup at the top and the unread marker line when (multiple) new messages are received

Sounds good to me, what do you think @roryabraham? I will make sure to create the regression test at the end.

roryabraham commented 2 months ago

@chrispader to clarify, are you proposing to show the new message indicator when any new messages appear in a chat you're currently viewing?

I'd think that we'd only want to show the new message indicator if you were offline and come back online to new messages you haven't received. Perhaps we should discuss in slack though

chrispader commented 2 months ago

Show both "New messages" popup at the top and the unread marker line when (multiple) new messages are received

@roryabraham Sorry for the confusion. Should have added "...while offline" at the end. I think we have the same approach in mind!

I wanted the "New messages" popup and unread marker to only be displayed when the user was offline or in another report

melvin-bot[bot] commented 2 months ago

๐Ÿ“ฃ It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? ๐Ÿ’ธ

brunovjk commented 2 months ago

Hey @chrispader, just checking inโ€”do you have everything needed to keep things moving forward, or is there anything I can assist with? Let me know if you need any help! Thank you :D

melvin-bot[bot] commented 2 months ago

๐Ÿ“ฃ @brunovjk ๐ŸŽ‰ An offer has been automatically sent to your Upwork account for the Reviewer role ๐ŸŽ‰ Thanks for contributing to the Expensify app!

Offer link Upwork job

roryabraham commented 2 months ago

sounds good @chrispader ๐Ÿ‘๐Ÿผ

chrispader commented 2 months ago

@brunovjk yes sorry for the delay. i have everything i need, was busy with other tasks until now, but going to fix this soon!

melvin-bot[bot] commented 1 month ago

@tylerkaraszewski, @alexpensify, @chrispader, @brunovjk Eep! 4 days overdue now. Issues have feelings too...

brunovjk commented 1 month ago

Not overdue, chrispader working on it.

alexpensify commented 1 month ago

@chrispader - any update here? Thanks!

alexpensify commented 1 month ago

No update