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

[Fabric] IOS - New Chat/New Group - The avatar jumps and flashes when opening a chat with a new user. #14365

Closed
1 task
dylanexpensify opened this issue Jan 17, 2023 · 15 comments
Assignees
Labels
Engineering NewFeature Something to build that is a new item. Weekly KSv2

Comments

@dylanexpensify
Copy link
Contributor

Action Performed:

  1. Log in to NewDot with any type of account
  2. Click on the green + button
  3. Click on the "New Chat" option
  4. Search for a user that you don't have any messages with and open the chat.

Expected Result:

Transaction should be smooth when opening a new group

Actual Result:

The avatar jumps and flashes when opening a chat with a new user.

Workaround:

Unknown

Platforms:

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

  • IOS / native

Version Number: 1.2.49.0

Reproducible in staging?: Yes

Reproducible in production?: No

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

Notes/Photos/Videos: Any additional supporting documentation

Bug5887845_jump_ios_0601.mp4

Expensify/Expensify Issue URL:

Issue reported by: Applause - Internal Team

Slack conversation:

View all open jobs on GitHub

@dylanexpensify dylanexpensify added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Jan 17, 2023
@melvin-bot melvin-bot bot locked and limited conversation to collaborators Jan 17, 2023
@dylanexpensify
Copy link
Contributor Author

dylanexpensify commented Jan 17, 2023

Coming from here. Context here!

@Expensify Expensify unlocked this conversation Jan 17, 2023
@roryabraham roryabraham added Engineering NewFeature Something to build that is a new item. and removed Bug Something is broken. Auto assigns a BugZero manager. labels Jan 17, 2023
@trjExpensify
Copy link
Contributor

I wonder how much this is down to caching avatars, @roryabraham? 🤔

@roryabraham roryabraham changed the title IOS - New Chat/New Group - The avatar jumps and flashes when opening a chat with a new user. [Fabric] IOS - New Chat/New Group - The avatar jumps and flashes when opening a chat with a new user. Jan 18, 2023
@roryabraham
Copy link
Contributor

First things first, I clarified that this issue is only happening on the Fabric branch by adding the [Fabric] prefix to the issue title. That seems worth doing for all the Fabric-only issues.

I wonder how much this is down to caching avatars, @roryabraham? 🤔

Doesn't look related to avatar caching to me. If I had to guess, I would guess that this is happening when the CREATED action is updated to be "finalized":

  1. Report opens with optimistic CREATED action (w/ `pendingAction: 'add')
  2. OpenReport API command sent.
  3. The API command completes, and the pendingAction field of the CREATED action is set to null. This causes ReportActionItem to re-render, which is causing a flash for some unknown reason.

@roryabraham
Copy link
Contributor

Looks like @WoLewicki may be on the case of this bug already: https://expensify.slack.com/archives/C04878MDF34/p1673978560875489

@trjExpensify
Copy link
Contributor

First things first, I clarified that this issue is only happening on the Fabric branch by adding the [Fabric] prefix to the issue title. That seems worth doing for all the Fabric-only issues.

Yes, great idea! Let's communicate that somewhere more widely. We could even be more explicit with [Fabric-only] as it might get mixed up with simply noting it's part of the project.

Doesn't look related to avatar caching to me

Okay, great. Sounds good!

@WoLewicki
Copy link
Contributor

I am working on this issue now.

@JmillsExpensify
Copy link

Nice! Assigned @WoLewicki as well.

@melvin-bot melvin-bot bot added the Overdue label Jan 20, 2023
@JmillsExpensify JmillsExpensify added Weekly KSv2 and removed Daily KSv2 labels Jan 21, 2023
@melvin-bot melvin-bot bot removed the Overdue label Jan 21, 2023
@JmillsExpensify
Copy link

Moved to weekly since that's the speed this project is moving at.

@j-piasecki
Copy link
Contributor

This issue was caused by the reportActionID of action with type CREATED being different each time the chat was opened, causing rerender of that component. This has been fixed on the main branch and merged into the one with fabric integration.

@melvin-bot melvin-bot bot added the Overdue label Jan 30, 2023
@JmillsExpensify
Copy link

@roryabraham @trjExpensify It'd be good to align on what our ideal process here from here on out for Fabric improvement issues like this one, where the fix is already on the main branch?

@melvin-bot melvin-bot bot removed the Overdue label Jan 31, 2023
@trjExpensify
Copy link
Contributor

Oh, so what happened? There was actually a bug on main that was causing this?

@roryabraham
Copy link
Contributor

Not exactly. What was happening was:

  • Optimistic CREATED action was shown
  • PHP hack inserted CREATED action with a random ID
  • Created action is received by front-end with new ID
  • New reportActionID causes component to re-render (because reportActionID is the key of the item in FlatList)
  • UI flashes on Fabric (unclear why)

The migration that added CREATED actions to the DB fixed this problem because:

  • Optimistic CREATED action is shown with client-generated canonical reportActionID
  • Since the reportActionID remains the same, there is no re-render caused, and therefore no flash

@roryabraham
Copy link
Contributor

I think we can close this now.

@melvin-bot
Copy link

melvin-bot bot commented Feb 1, 2023

@JmillsExpensify Be sure to fill out the Contact List!

@JmillsExpensify
Copy link

Say what? Not sure what that's referring to.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Engineering NewFeature Something to build that is a new item. Weekly KSv2
Projects
None yet
Development

No branches or pull requests

6 participants