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

[$500] Chat - Loading animation not shown when open 1:1 Conversation with uncached attachments #32699

Closed
5 of 6 tasks
izarutskaya opened this issue Dec 7, 2023 · 73 comments
Closed
5 of 6 tasks
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Engineering External Added to denote the issue can be worked on by a contributor Needs Reproduction Reproducible steps needed Not a priority

Comments

@izarutskaya
Copy link

izarutskaya commented Dec 7, 2023

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


Found when executing PR: #13036

Version Number: v1.4.9-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:
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:

Precondition: user should be signed in

  1. Open the app
  2. Send several type attachments from the other account on main device
  3. Disable internet connection
  4. Open the 1:1 Conversation (attachments should be uncached)

Expected Result:

The attachment previews should be empty with infinite loading indicator because the attachment were uncached and user is offline.

Actual Result:

The attachments should appear in an infinite loading state, indicating they cannot be fetched without an internet connection

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

RPReplay_Final1701973445.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01d5ec534790c61159
  • Upwork Job ID: 1732883412213059584
  • Last Price Increase: 2024-01-04
Issue OwnerCurrent Issue Owner: @mallenexpensify
@izarutskaya izarutskaya added External Added to denote the issue can be worked on by a contributor Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Dec 7, 2023
Copy link

melvin-bot bot commented Dec 7, 2023

Triggered auto assignment to @JmillsExpensify (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

@melvin-bot melvin-bot bot changed the title Chat - Loading animation not shown when open 1:1 Conversation with uncached attachments [$500] Chat - Loading animation not shown when open 1:1 Conversation with uncached attachments Dec 7, 2023
Copy link

melvin-bot bot commented Dec 7, 2023

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

Copy link

melvin-bot bot commented Dec 7, 2023

Bug0 Triage Checklist (Main S/O)

  • This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
  • This bug is not a duplicate report (check E/App issues and #expensify-bugs)
    • If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
  • This bug is reproducible using the reproduction steps in the OP. S/O
    • If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
    • If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
  • This issue is filled out as thoroughly and clearly as possible
    • Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
  • I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

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

melvin-bot bot commented Dec 7, 2023

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

@ikevin127

This comment was marked as outdated.

@tienifr
Copy link
Contributor

tienifr commented Dec 8, 2023

Proposal

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

Chat - Loading animation not shown when open 1:1 Conversation with uncached attachments

What is the root cause of that problem?

We're using isLoading and isImageCached to show the loading

{isLoading && !isImageCached && <FullscreenLoadingIndicator style={[styles.opacity1, styles.bgTransparent]} />}

but we update these states in onLoadEnd callback

This callback will be invoked when load either succeeds or fails

so when the images load fails, isLoading is false -> there's no loading indicator

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

Solution 1: Show the loading for all error images

We should move this logic to imageLoadedSuccessfully

then trigger onLoad when network.isOffline is false

    useEffect(() => {
        // If an onLoad callback was specified then manually call it and pass
        // the natural image dimensions to match the native API
        if (onLoad == null || isOffline) {
            return;
        }
        RNImage.getSize(source.uri, (width, height) => {
            onLoad({nativeEvent: {width, height}});
        });
    }, [onLoad, source, isOffline]);

And update other components that faces this issue as well

Solution 2: Show the loading for the error images while offline

Do the same as the solution 1, but in onError we should add the logic to handle error image

  const onError = () => {
    Log.hmmm('Unable to fetch image to calculate size', {url});
     if(isOffline){
         return;
     }
    setIsLoading(false);
    setIsImageCached(true);
  };

Result

Screen.Recording.2023-12-08.at.11.46.02.mov

@ikevin127
Copy link
Contributor

Cannot reproduce the issue following the given steps.

@melvin-bot melvin-bot bot added the Overdue label Dec 11, 2023
Copy link

melvin-bot bot commented Dec 11, 2023

@JmillsExpensify, @getusha Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

@getusha
Copy link
Contributor

getusha commented Dec 11, 2023

@izarutskaya @tienifr i am unable to reproduce could you confirm?

@melvin-bot melvin-bot bot removed the Overdue label Dec 11, 2023
@JmillsExpensify
Copy link

Waiting for confirmation of reproduction.

Copy link

melvin-bot bot commented Dec 14, 2023

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

@melvin-bot melvin-bot bot added the Overdue label Dec 15, 2023
Copy link

melvin-bot bot commented Dec 18, 2023

@JmillsExpensify, @getusha Eep! 4 days overdue now. Issues have feelings too...

@getusha
Copy link
Contributor

getusha commented Dec 18, 2023

Friendly bump @izarutskaya, is this still reproducible?

@melvin-bot melvin-bot bot added Overdue and removed Overdue labels Dec 18, 2023
@tienifr
Copy link
Contributor

tienifr commented Dec 21, 2023

@getusha I can reproduce. Please disable cache and network on browser

Before:

Screen.Recording.2023-12-21.at.11.03.08.mp4

After applying solution:

Screen.Recording.2023-12-21.at.11.02.05.mp4

Copy link

melvin-bot bot commented Dec 21, 2023

@JmillsExpensify @getusha this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

Copy link

melvin-bot bot commented Dec 21, 2023

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

@getusha
Copy link
Contributor

getusha commented Dec 21, 2023

Testing @tienifr's proposal

@JmillsExpensify
Copy link

$500 approved for @getusha

@tienifr
Copy link
Contributor

tienifr commented Jul 24, 2024

Thank you @mallenexpensify I accepted the sent offer.

@JmillsExpensify
Copy link

@tienifr you won't be paid via NewDot, correct?

@mallenexpensify
Copy link
Contributor

mallenexpensify commented Jul 29, 2024

@tienifr you won't be paid via NewDot, correct?

Correct, they were assigned forever ago

yuwenmemon assigned tienifr on Jan 29

@tienifr paid, payment post updated above.

@getusha same for you, you were hired last year :/

@melvin-bot melvin-bot bot assigned getusha on Dec 7, 2023

@getusha can you please accept the job and reply here once you have?
https://www.upwork.com/jobs/~018596a1cbe0c4659c

You already received the NewDot payment from Jason, right? We'll cancel it out with another issue/job, if so.

@mallenexpensify mallenexpensify added Daily KSv2 and removed Reviewing Has a PR in review Monthly KSv2 labels Jul 30, 2024
@mallenexpensify mallenexpensify self-assigned this Jul 30, 2024
@melvin-bot melvin-bot bot removed the Overdue label Jul 30, 2024
@getusha
Copy link
Contributor

getusha commented Aug 11, 2024

You already received the NewDot payment from Jason, right? We'll cancel it out with another issue/job, if so.

Yes

@mallenexpensify
Copy link
Contributor

@getusha let's use this $250 one to start, then we'll find another $250 issue with the hire date after your eligibility date, 2024-05-25

@mallenexpensify mallenexpensify added Weekly KSv2 Needs Reproduction Reproducible steps needed Awaiting Payment Auto-added when associated PR is deployed to production and removed Daily KSv2 labels Aug 14, 2024
@MelvinBot
Copy link

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

@mallenexpensify
Copy link
Contributor

Assigned myself as the owner and bumping to weekly pending when payment due on the below

@getusha
Copy link
Contributor

getusha commented Aug 19, 2024

@mallenexpensify could you end the UW contract? ty!

@mallenexpensify
Copy link
Contributor

Done, contract ended @getusha

@melvin-bot melvin-bot bot added Daily KSv2 and removed Weekly KSv2 labels Aug 21, 2024
@getusha
Copy link
Contributor

getusha commented Aug 24, 2024

@mallenexpensify the contract ended but strangely i didn't receive the payment

Screenshot 2024-08-24 at 11 37 03 in the morning

@melvin-bot melvin-bot bot added the Overdue label Aug 24, 2024
@mallenexpensify
Copy link
Contributor

All set @getusha , sorry about that
image

Updated the payment post above too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Engineering External Added to denote the issue can be worked on by a contributor Needs Reproduction Reproducible steps needed Not a priority
Projects
None yet
Development

No branches or pull requests