[HOLD for payment 2024-11-29] [$250] Attachment - Video player buttons unresponsive to single click in fullscreen after expand

[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.52-1
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught on HybridApp, is this reproducible on New Expensify Standalone?: N/A
If this was caught during regression testing, add the test name, ID and link from TestRail: N/A
Issue reported by: Applause - Internal Team

Action Performed:

1. Open https://staging.new.expensify.com/
2. Navigate to existing chat with video attachment
3. Play the video
4. Click on Expand in top right corner of video preview
5. Click on Fullscreen
6. Click on Fullscreen again

Expected Result:

Video should exit full screen mode and should be expanded

Actual Result:

Video remains in full screen mode. Player buttons are unresponsive to single mouse button click
Buttons work if double clicked or clicking rapidly

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

Bug6641955_1729596126438.2024-10-22_13-05-40.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~021851667925664029279
• Upwork Job ID: 1851667925664029279
• Last Price Increase: 2024-10-30
• Automatic offers:
• akinwale | Reviewer | 104770436
• QichenZhu | Contributor | 104770439

Issue Owner

Current Issue Owner: @JmillsExpensify

[melvin-bot]

Triggered auto assignment to @JmillsExpensify (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]

@JmillsExpensify 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

[QichenZhu]

Edited by proposal-police: This proposal was edited at 2024-11-04 11:29:24 UTC.

Proposal

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

Controls of the fullscreen video player are not clickable while the mouse is moving.

What is the root cause of that problem?

If you move the mouse even a little when clicking, the control doesn't work, because the pointermove event bubbles up, and PointerEventManager sets the player as the capture target, meaning that subsequent events will be targeted at the player until the mouse is released. But the real target should be the control button inside the player's shadow DOM.

App/src/components/Attachments/AttachmentCarousel/index.tsx

Line 288 in f46bce0

<GestureDetector gesture={pan}>

https://github.com/software-mansion/react-native-gesture-handler/blob/802da495101cd3fbf2257e2bf7271fe1719b6638/src/web/tools/PointerEventManager.ts#L104

If you click without moving the mouse, the control works fine, because in this case the browser doesn't bubble the pointerdown event to PointerEventManager.

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

1. Conditionally renderGestureDetector in src/components/Attachments/AttachmentCarousel/index.tsx.

const MaybeGestureDetector = ({canUseTouchScreen, gesture, children}) => (
    canUseTouchScreen ? (
        <GestureDetector gesture={gesture}>
            {children}
        </GestureDetector>
    ) : children
);

<MaybeGestureDetector canUseTouchScreen={canUseTouchScreen} gesture={pan}>
    // L289-L306
</MaybeGestureDetector>

2. pagerRef is null if GestureDetector is not rendered. In case it causes issues, they’ll be handled in the PR.

What alternative solutions did you explore? (Optional)

Add VIDEO tag to the exclusion list of capture targets alongside SELECT and INPUT below.

https://github.com/software-mansion/react-native-gesture-handler/blob/802da495101cd3fbf2257e2bf7271fe1719b6638/src/web/tools/PointerEventManager.ts#L12

Not sure if it’s a good idea to modify the library, as it may affect library users who rely on this feature, though I don’t see an obvious benifit of using VIDEO tag as a capture target in our repo.

[melvin-bot]

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

[melvin-bot]

@JmillsExpensify Still overdue 6 days?! Let's take care of this!

[JmillsExpensify]

Opening this up to the community.

[melvin-bot]

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