[HOLD for payment 2024-03-07] [$500] Video - Unable to adjust volume by clicking on any part of the volume slider

[lanitochka17]

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.4.43-3
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
2. Go to any chat
3. Upload a video
4. In the preview, click on the volume icon
5. Click on the volume slider to adjust

Expected Result:

User should be able to adjust volume by clicking on any length of the volume slider

Actual Result:

Video player volume does not respond until user holds and drags across the slider

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

Bug6386034_1708454719781.20240220_231408.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01ddb8fc91ce5926ec
• Upwork Job ID: 1760051536709566464
• Last Price Increase: 2024-02-20
• Automatic offers:
• cubuspl42 | Reviewer | 0

[melvin-bot]

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

[melvin-bot]

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

[github-actions]

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

[melvin-bot]

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

[lanitochka17]

We think that this bug might be related to #vip-vsp
CC @quinthar

[Krishna2323]

Proposal

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

Video - Unable to adjust volume by clicking on any part of the volume slider

What is the root cause of that problem?

We don't handle tap gesture for increasing/decreasing volume.

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

We need to add tab gesture and compose it with pan gesture.

    const pan = Gesture.Pan()
        .onBegin(() => {
            runOnJS(setIsSliderBeingUsed)(true);
        })
        .onChange((event) => {
            const val = Math.floor((1 - event.y / sliderHeight) * 100) / 100;
            volume.value = Math.min(Math.max(val, 0), 1);
        })
        .onEnd(() => {
            runOnJS(setIsSliderBeingUsed)(false);
        });

    const tapGesture = Gesture.Tap().onEnd((event) => {
        const val = Math.floor((1 - event.y / sliderHeight) * 100) / 100;
        volume.value = Math.min(Math.max(val, 0), 1);
    });

    const composed = Gesture.Race(pan, tapGesture);

PS: Minor changes can be made through PR phase.

Result

tap_gesture.mp4

[roryabraham]

def-o related to #30829

[roryabraham]

I think that:

• since this is a bug with the new video player feature
• we're not planning to deploy NewDot to prod this week

we should let @Skalakid and SWM take point on fixing this issue. Making it a daily but leaving it as a deploy blocker (weird combo I know but we've got a merge freeze in place)

[francoisl]

cc @kowczarz

[kowczarz]

I will take a look on this

[kowczarz]

The root cause in this proposal is fine #36952 (comment), but composing is IMO an overkill.
Instead I would add additional handler on begin pan event:

    const gestureEventHandler = (event) => {
        const val = Math.floor((1 - event.y / sliderHeight) * 100) / 100;
        volume.value = Math.min(Math.max(val, 0), 1);
     }
    const pan = Gesture.Pan()
        .onBegin((event) => {
            runOnJS(setIsSliderBeingUsed)(true);
            gestureEventHandler(event)
        })
        .onChange((event) => {
            gestureEventHandler(event);
        })
        .onEnd(() => {
            runOnJS(setIsSliderBeingUsed)(false);
        });

[Krishna2323]

@kowczarz, yeah that's a great idea :)

[melvin-bot]

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

Offer link
Upwork job

[roryabraham]

PR awaiting review from @cubuspl42

[melvin-bot]

Reviewing label has been removed, please complete the "BugZero Checklist".

[melvin-bot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.4.45-6 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• Fix unable to adjust volume by clicking on any part of the volume slider #37094

If no regressions arise, payment will be issued on 2024-03-07. 🎊

For reference, here are some details about the assignees on this issue:

• @cubuspl42 requires payment automatic offer (Reviewer)
• @kowczarz does not require payment (Contractor)

[melvin-bot]

Issue is ready for payment but no BZ is assigned. @garrettmknight you are the lucky winner! Please verify the payment summary looks correct and complete the checklist. Thanks!

[garrettmknight]

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

• [@cubuspl42] The PR that introduced the bug has been identified. Link to the PR:
• [@cubuspl42] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
• [@cubuspl42] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
• [@cubuspl42] Determine if we should create a regression test for this bug.
• [@cubuspl42] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
• [@garrettmknight] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[garrettmknight]

@cubuspl42 all paid - can you complete the checklist?

[cubuspl42]

• The PR that introduced the bug has been identified. Link to the PR:
  • N/A; more like a new feature/case
• The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
  • N/A
• A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
  • No need for additional discussion
• Determine if we should create a regression test for this bug.
  • No
• If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
  • N/A