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.

Sign up for GitHub

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

Jump to bottom

Fix: Context menu not appear on long press video #36940

Merged
merged 4 commits into from
Feb 21, 2024
Merged

Fix: Context menu not appear on long press video #36940

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6d7d9b8
fix: 36868
tienifr Feb 20, 2024
7ebee9a
fix lint
tienifr Feb 20, 2024
cd6b1a8
fix lint
tienifr Feb 20, 2024
84fc9c2
fix: long press video toggles text selection
tienifr Feb 20, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 29 additions & 16 deletions src/components/VideoPlayerPreview/VideoPlayerThumbnail.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,11 @@ import Icon from '@components/Icon';
import * as Expensicons from '@components/Icon/Expensicons';
import Image from '@components/Image';
import PressableWithoutFeedback from '@components/Pressable/PressableWithoutFeedback';
import {ShowContextMenuContext, showContextMenuForReport} from '@components/ShowContextMenuContext';
import useThemeStyles from '@hooks/useThemeStyles';
import ControlSelection from '@libs/ControlSelection';
import * as DeviceCapabilities from '@libs/DeviceCapabilities';
import * as ReportUtils from '@libs/ReportUtils';
import variables from '@styles/variables';
import CONST from '@src/CONST';

Expand Down Expand Up @@ -35,22 +39,31 @@ function VideoPlayerThumbnail({thumbnailUrl, onPress, accessibilityLabel}) {
/>
</View>
)}
<PressableWithoutFeedback
style={[styles.videoThumbnailContainer]}
accessibilityLabel={accessibilityLabel}
accessibilityRole={CONST.ACCESSIBILITY_ROLE.BUTTON}
onPress={onPress}
>
<View style={[styles.videoThumbnailPlayButton]}>
<Icon
src={Expensicons.Play}
fill="white"
width={variables.iconSizeXLarge}
height={variables.iconSizeXLarge}
additionalStyles={[styles.ml1]}
/>
</View>
</PressableWithoutFeedback>
<ShowContextMenuContext.Consumer>
{({anchor, report, action, checkIfContextMenuActive}) => (
<PressableWithoutFeedback
style={[styles.videoThumbnailContainer]}
accessibilityLabel={accessibilityLabel}
accessibilityRole={CONST.ACCESSIBILITY_ROLE.BUTTON}
onPress={onPress}
onPressIn={() => DeviceCapabilities.canUseTouchScreen() && ControlSelection.block()}
onPressOut={() => ControlSelection.unblock()}
Copy link
Contributor Author

@tienifr tienifr Feb 20, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Without onPressIn and onPressOut here, long pressing video thumbnail would toggle text selection (i.e. the magnifier) on mWeb:

Screen.Recording.2024-02-21.at.02.51.45-compressed.mov

This has already been used throughout the App like here.

onLongPress={(event) =>
showContextMenuForReport(event, anchor, (report && report.reportID) || '', action, checkIfContextMenuActive, ReportUtils.isArchivedRoom(report))
}
>
<View style={[styles.videoThumbnailPlayButton]}>
<Icon
src={Expensicons.Play}
fill="white"
width={variables.iconSizeXLarge}
height={variables.iconSizeXLarge}
additionalStyles={[styles.ml1]}
/>
</View>
</PressableWithoutFeedback>
)}
</ShowContextMenuContext.Consumer>
</View>
);
}
Expand Down
Loading