[$250] Track - Loading spinner appears on a Track your expenses video modal

[lanitochka17]

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.29-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: N/A
Email or phone of affected tester (no customers): gocemate+a2069@gmail.com
Issue reported by: Applause - Internal Team

Action Performed:

1. Go to FAB> Track
2. Verify that video modal appears

Expected Result:

There should be no loading spinner over the video modal

Actual Result:

Loading spinner appears on a Track your expenses video modal and remains to the end of the video

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

Add any screenshot/video evidence

Bug6593462_1725483841193.Recording__3883.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~021832057700758749301
• Upwork Job ID: 1832057700758749301
• Last Price Increase: 2024-09-20
• Automatic offers:
• HezekielT | Contributor | 104122623

Issue Owner

Current Issue Owner: @hungvu193

[melvin-bot]

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

[lanitochka17]

@bfitzexpensify FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors

[nyomanjyotisa]

Edited by proposal-police: This proposal was edited at {current_timestamp}.

Proposal

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

Loading spinner appears on a Track your expenses video modal

What is the root cause of that problem?

The loading spinner is controlled by setIsLoading and relies on status.durationMillis here. And sometime the expo-av video player might not be providing valid values for durationMillis

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

Update the setIsLoading logic to handle both durationMillis and positionMillis, only set isLoading to true if status.durationMillis is NaN and status.positionMillis is 0

setIsLoading(Number.isNaN(status.durationMillis) && status.positionMillis === 0);

This ensures that the spinner disappears when the video starts playing

What alternative solutions did you explore? (Optional)

[melvin-bot]

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

[melvin-bot]

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

[bfitzexpensify]

I am heading out of office until September 21st, so assigning a buddy to watch over this in my absence.

Current status: waiting on proposals

[melvin-bot]

Current assignee @bfitzexpensify is eligible for the Bug assigner, not assigning anyone new.

[hungvu193]

@nyomanjyotisa Are you sure?
If you used nullish coalescing operator, it's only return the right-hand side operand if the left-hand side is null or undefined.

setIsLoading(Number.isNaN(status.durationMillis) ?? status.positionMillis > 0);

is the same with

setIsLoading(Number.isNaN(status.durationMillis) )

Beacuse Number.isNaN return bool.

[shubham1206agra]

This looks super weird to me. @bfitzexpensify Can we ask someone from an expert agency to look into this to avoid a workaround?

[nyomanjyotisa]

Proposal updated

[hungvu193]

I'll take closer look today.

[hungvu193]

Can't reproduce on latest main:

Screen.Recording.2024-09-12.at.09.58.09.mov

[melvin-bot]

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

[hungvu193]

Can you retest this one @lanitochka17 ?

[melvin-bot]

@hungvu193 @bfitzexpensify 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!

[melvin-bot]

@hungvu193, @bfitzexpensify Whoops! This issue is 2 days overdue. Let's get this updated quick!

[hungvu193]

little bump @bfitzexpensify

[melvin-bot]

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

[melvin-bot]

@hungvu193, @bfitzexpensify Whoops! This issue is 2 days overdue. Let's get this updated quick!

[hungvu193]

little bump @bfitzexpensify

Same

[bfitzexpensify]

Apologies, just coming back from ooo

Have asked QA to retest

[m-natarajan]

Able to reproduce

Recording.4029.mp4

[hungvu193]

Interesting, I'll have another look today

[HezekielT]

Proposal

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

Track - Loading spinner appears on a Track your expenses video modal and remains to the end of the video

What is root cause of that problem?

We show the loading indicator if it is buffering in addition to checking isLoading and isOffline.

App/src/components/VideoPlayer/BaseVideoPlayer.tsx

Line 444 in dc74ee3

{((isLoading && !isOffline) || isBuffering) && <FullScreenLoadingIndicator style={[styles.opacity1, styles.bgTransparent]} />}

But in android, the isBuffering variable is true instead of becoming false even after the video has already started playing which is why the loading indicator doesn’t disappear. This was reported as issue here

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

One of the comment suggests that the video will play even if the buffering hasn’t finished completely and as such we should use both isBuffering and isPlaying variable while determining when to show the loading indicator.

The official documentation also advises to do the same.

Playback may not start immediately after calling this function for reasons such as buffering. Make sure to update your UI based on the isPlaying and isBuffering properties of the AVPlaybackStatus

Therefore, in order to solve this issue, we should update the following line

App/src/components/VideoPlayer/BaseVideoPlayer.tsx

Line 444 in dc74ee3

{((isLoading && !isOffline) || isBuffering) && <FullScreenLoadingIndicator style={[styles.opacity1, styles.bgTransparent]} />}

By adding !isPlaying as shown below.

- {((isLoading && !isOffline) || isBuffering) && <FullScreenLoadingIndicator style={[styles.opacity1, styles.bgTransparent]} />}
+ {((isLoading && !isOffline) || (isBuffering && !isPlaying)) && <FullScreenLoadingIndicator style={[styles.opacity1, styles.bgTransparent]} />}

What alternative solutions did you explore? (Optional)

None.


Result

android-fix-loading-video.mp4

[github-actions]

@HezekielT Your proposal will be dismissed because you did not follow the proposal template.

[hungvu193]

I managed to reproduce on my Pixel 4a. I'll review proposals today

[hungvu193]

Thanks for the proposals, everyone. It looks like on Android, sometimes the isBuffering is set incorrectly

Android buffering

Screen.Recording.2024-09-25.at.14.16.17.mov

I'd go with @HezekielT 's proposal that it has the correct RCA and I agree we shouldn't show the loading indicator while it's playing.

🎀 👀 🎀 C+ reviewed.

[melvin-bot]

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

[MarioExpensify]

@HezekielT proposal LGTM! Thanks @HezekielT and @hungvu193

[melvin-bot]

📣 @HezekielT 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job
Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖

[HezekielT]

@hungvu193 PR is ready!