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

Start chat - List does not scroll to the top after adding a few users to group #55132

Closed
2 of 8 tasks
lanitochka17 opened this issue Jan 11, 2025 · 20 comments
Closed
2 of 8 tasks
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Engineering

Comments

@lanitochka17
Copy link

lanitochka17 commented Jan 11, 2025

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: 9.0.84-1
Reproducible in staging?: Y
Reproducible in production?: N
If this was caught on HybridApp, is this reproducible on New Expensify Standalone?: Y
If this was caught during regression testing, add the test name, ID and link from TestRail: N/A
Email or phone of affected tester (no customers): applausetester+110106kh@applause.expensifail.com
Issue reported by: Applause - Internal Team

Action Performed:

  1. Launch ND or hybrid app
  2. Open FAB > Start chat
  3. Scroll down the list
  4. Tap Add to group on any user
  5. Scroll down and tap Add to group again
  6. Scroll down and tap Add to group again

Expected Result:

  • When app scrolls up after tapping Add to group, the contact will be instantly marked with a checkmark
  • In Step 6, after adding third member to group, app will scroll to the top of the list

Actual Result:

  • When app scrolls up after tapping Add to group, app takes some seconds before the contact is instantly marked with a checkmark (on Expensifail accounts)
  • In Step 6, after adding third member to group, app does not scroll to the top of the list. It only scrolls up to show two selected members. User will have to scroll up to show the third user

Workaround:

Unknown

Platforms:

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

  • Android: Standalone
  • Android: HybridApp
  • Android: mWeb Chrome
  • iOS: Standalone
  • iOS: HybridApp
  • iOS: mWeb Safari
  • MacOS: Chrome / Safari
  • MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence
Bug6712076_1736627825406.ScreenRecording_01-12-2025_04-31-44_1.1.mp4

View all open jobs on GitHub

@lanitochka17 lanitochka17 added the DeployBlockerCash This issue or pull request should block deployment label Jan 11, 2025
Copy link

melvin-bot bot commented Jan 11, 2025

Triggered auto assignment to @blimpich (DeployBlockerCash), see https://stackoverflowteams.com/c/expensify/questions/9980/ for more details.

Copy link

melvin-bot bot commented Jan 11, 2025

💬 A slack conversation has been started in #expensify-open-source

Copy link
Contributor

👋 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.

@ishpaul777
Copy link
Contributor

most probably coming from this PR #54994

@daledah
Copy link
Contributor

daledah commented Jan 13, 2025

I think this is not a regression, but indeed a different bug, as this bug appears on workspace invite page as well:

Screen.Recording.2025-01-13.at.14.15.04.mov

And my PR only apply changes to new chat page.
I can't reproduce the first bug though.

@dukenv0307
Copy link
Contributor

@lanitochka17 @blimpich

I can't reproduce the first bug

When app scrolls up after tapping Add to group, the contact will be instantly marked with a checkmark

Can you check with WS invite page?

In Step 6, after adding third member to group, app will scroll to the top of the list

This issue happens for other pages as well, and I don't think it's an actual bug. Let's assume we have the very long selected list, users expect to see the latest added users (not the first one).

@Beamanator
Copy link
Contributor

Added my comments in slack, In my opinion we should call this NAB - for similar reasons above.

FYI i believe the scroll / checkmark lag are due to applause's super high traffic accounts

@blimpich blimpich added Daily KSv2 and removed DeployBlockerCash This issue or pull request should block deployment Hourly KSv2 labels Jan 13, 2025
@blimpich
Copy link
Contributor

I agree with @Beamanator, not a blocker and lag is probably due to high traffic account.

@blimpich blimpich added the Bug Something is broken. Auto assigns a BugZero manager. label Jan 13, 2025
Copy link

melvin-bot bot commented Jan 13, 2025

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

@blimpich
Copy link
Contributor

blimpich commented Jan 13, 2025

@OfstadC this issue is really two sub-issues: (1) the app lags when selecting to "add to a group" in large high traffic accounts and (2) the amount that the app scrolls up upon selecting "add to group" is maybe wrong. Issues states this is the expected behavior:

Expected: after adding third member to group, app will scroll to the top of the list
Actual: after adding third member to group, app does not scroll to the top of the list. It only scrolls up to show two selected members. User will have to scroll up to show the third user

Part 1 is a quality bug and should probably be handled as such, but can you help us figure out the 2nd part and what is, in fact, the intended behavior?

@OfstadC
Copy link
Contributor

OfstadC commented Jan 15, 2025

Part 1 is a quality bug and should probably be handled as such, but can you help us figure out the 2nd part and what is, in fact, the intended behavior?

Sure! I can do some testing. I'm online 50% today but I should be able to get to this tomorrow

@OfstadC
Copy link
Contributor

OfstadC commented Jan 17, 2025

@blimpich I was able to reproduce:

It only scrolls up to show two selected members. User will have to scroll up to show the third user

corinne++2@ was the first user, then corinne+7

Image

Then I added corinne+6 which actually did auto-check and move to the top, but corinne++2 was no longer showing

Image

Until I scrolled up:

Image

To me, it feels odd for the third user specifically, but after adding 4, 5, 6 + users it makes sense. I think when scrolling down it's prioritizing showing the list of users/contacts, but once one is selected it's bringing you back to the text box to type a users name/email.

@melvin-bot melvin-bot bot added the Overdue label Jan 20, 2025
@blimpich
Copy link
Contributor

Yeah it feels odds but I don't know what exactly would feel right. Plus maybe it looks weird on mobile but not as much on desktop. @Expensify/design do you have any input here?

@melvin-bot melvin-bot bot removed the Overdue label Jan 20, 2025
@dubielzyk-expensify
Copy link
Contributor

Tested it locally and it feels a bit weird to me that we're scrolling to only show 2 if you've added more. I think if we scroll up after tapping Add to group we should be scrolling to the top always, regardless of how many have been added.

(Sidenote: I've always found the scroll-to-top when adding a member to be pretty jarring anyways at it requires you to scroll up and down and it's interrupting the users flow, but that's a bigger thing to solve for)

@shawnborton
Copy link
Contributor

I think if we scroll up after tapping Add to group we should be scrolling to the top always, regardless of how many have been added.

This is my general thoughts - we make a rule that we always scroll to the top after adding to a group, or we don't.

I have mixed feelings on this one - it certainly doesn't feel great, and part of that might be because of that floating referral banner that takes up so much vertical space. But I think the idea behind this was that if you were creating a group, you'd likely be using the search bar to search for members, at which point it made sense that you always floated back to the top since most of your interactions here start after you type a name.

But yeah, I tried it locally too and I think we should really be scrolling this the whole way up to the top since that's the current behavior of this flow.

@dubielzyk-expensify
Copy link
Contributor

I definitely agree with this. For the record, I wasn't trying to suggest some sort of conditional scrolling. If anything it almost seems like we're doing that at the moment.

But I think the idea behind this was that if you were creating a group, you'd likely be using the search bar to search for members, at which point it made sense that you always floated back to the top since most of your interactions here start after you type a name.

I can see where it's coming from, but this doesn't make heaps of sense to me as you're likely to not even need much of a "jump up" if you type it at the top cause your results are likely to be at the top as you type, vs if you scroll down the list and add manually. Anyways, I think we're on the same page.

For this bug though I still kinda think we should stick with the current behavior to avoid too much change but just scroll to the top instead of "almost the top" which seems to be the case in the video at least? Unless I'm missing something 😅

@shawnborton
Copy link
Contributor

Good points all around, but totally agree - in the very least, let's take it to the VERY TOP instead of kinda the top.

@dannymcclain
Copy link
Contributor

I also have pretty mixed feelings about the scroll behavior 😅 but for this one I am totally down to take you to the very top instead of kinda the top 👍

@blimpich
Copy link
Contributor

blimpich commented Jan 21, 2025

Great! Okay I'm going to split this issue into two, one for the scroll behavior change and one for the lagging since that is probably different root cause and needs a #quality focus.

@blimpich
Copy link
Contributor

Issue for behavior change: #55548

Issue for dealing with lag: #55554

Going to close this original issue since we have two separate issues now for dealing with these two separate problems.

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 Engineering
Projects
None yet
Development

No branches or pull requests

10 participants