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

Chat - Scrolling does not work if the user adds a message or clicks 'New messages' button #30947

Closed
2 of 6 tasks
kbecciv opened this issue Nov 7, 2023 · 12 comments
Closed
2 of 6 tasks
Assignees

Comments

@kbecciv
Copy link

kbecciv commented Nov 7, 2023

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!


Version Number: 1.3.96.0
Reproducible in staging?: y
Reproducible in production?: n
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: Applause - Internal Team
Slack conversation:

Action Performed:

  1. Go to staging.new.expensify.com or open Desktop App
  2. Login with any account and go to chat with long history
  3. Scroll to the beginning of the chat
  4. Post new messages > Check the result
  5. Mark any message as unread and scroll up the chat until the "New messages" button appears
  6. Click the "New messages" button > Check the result

Expected Result:

  1. Message added and chat scrolled to new messages
  2. Chat scrolled to new messages

Actual Result:

  1. Message added and chat did not scroll to new messages
  2. Chat did not scroll to new messages

Workaround:

Unknown

Platforms:

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

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

Screenshots/Videos

Bug6266738_1699321983202.Chat-Button-New-Message-scroll-not-working.mp4

View all open jobs on GitHub

@kbecciv kbecciv added the DeployBlockerCash This issue or pull request should block deployment label Nov 7, 2023
@OSBotify
Copy link
Contributor

OSBotify commented Nov 7, 2023

👋 Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open StagingDeployCash deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:

  1. Identify the pull request that introduced this issue and revert it.
  2. Find someone who can quickly fix the issue.
  3. Fix the issue yourself.

Copy link

melvin-bot bot commented Nov 7, 2023

Triggered auto assignment to @thienlnam (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

@thienlnam
Copy link
Contributor

Hmm yeah, the 'New Message' indicator doesn't scroll me anymore

@situchan
Copy link
Contributor

situchan commented Nov 7, 2023

This came from custom ref handling in #28793
cc: @perunt @janicduplessis @thienlnam

@perunt
Copy link
Contributor

perunt commented Nov 7, 2023

If you press this button without navigating to thread would it scroll you down?

@situchan
Copy link
Contributor

situchan commented Nov 7, 2023

If you press this button without navigating to thread would it scroll you down?

Nope. Seems like ref of MVCPFlatList is not forwarded correctly

@situchan
Copy link
Contributor

situchan commented Nov 7, 2023

const scrollToIndex = (index: number, isEditing?: boolean) => {
if (!flatListRef?.current || isEditing) {
return;
}
flatListRef.current.scrollToIndex({index, animated: true});
};

Early returned because flatListRef.current is not set

@thienlnam
Copy link
Contributor

@perunt You around to get a PR up for this? Ideally we can just fix the ref forwarding and then we won't have to revert the PR

@situchan
Copy link
Contributor

situchan commented Nov 7, 2023

There's another similar regression: #30935

@situchan
Copy link
Contributor

situchan commented Nov 7, 2023

Confirmed #30962 fixes this issue as well

@thienlnam
Copy link
Contributor

Great - we'll link that one there then. Thanks for the help here!

@thienlnam
Copy link
Contributor

This was taken care of in #30962

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants