Skip to content

Commit

Permalink
Fix not showing duration while loading
Browse files Browse the repository at this point in the history
  • Loading branch information
kowczarz committed Feb 8, 2024
1 parent 7a858b7 commit 699e302
Show file tree
Hide file tree
Showing 6 changed files with 18 additions and 3 deletions.
3 changes: 3 additions & 0 deletions src/components/Attachments/AttachmentCarousel/CarouselItem.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ const propTypes = {

/** The id of the transaction related to the attachment */
transactionID: PropTypes.string,

duration: PropTypes.number,
}).isRequired,

/** Whether there is only one element in the attachment carousel */
Expand Down Expand Up @@ -118,6 +120,7 @@ function CarouselItem({item, index, activeIndex, isSingleItem, onPress, isFocuse
transactionID={item.transactionID}
isHovered={isModalHovered}
isFocused={isFocused}
optionalVideoDuration={item.duration}
/>
</View>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ function extractAttachmentsFromReport(parentReportAction, reportActions) {
source: tryResolveUrlFromApiRoot(attribs[CONST.ATTACHMENT_SOURCE_ATTRIBUTE]),
isAuthTokenRequired: Boolean(attribs[CONST.ATTACHMENT_SOURCE_ATTRIBUTE]),
file: {name: splittedUrl[splittedUrl.length - 1]},
duration: Number(attribs[CONST.ATTACHMENT_DURATION_ATTRIBUTE]),
isReceipt: false,
hasBeenFlagged: false,
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,21 +11,25 @@ const propTypes = {
isHovered: PropTypes.bool,

shouldUseSharedVideoElement: PropTypes.bool,

videoDuration: PropTypes.number,
};

const defaultProps = {
isHovered: false,
shouldUseSharedVideoElement: false,
videoDuration: 0,
};

function AttachmentViewVideo({source, isHovered, shouldUseSharedVideoElement}) {
function AttachmentViewVideo({source, isHovered, shouldUseSharedVideoElement, videoDuration}) {
const {isSmallScreenWidth} = useWindowDimensions();

return (
<VideoPlayer
url={source}
shouldUseSharedVideoElement={shouldUseSharedVideoElement && !isSmallScreenWidth}
isVideoHovered={isHovered}
videoDuration={videoDuration}
/>
);
}
Expand Down
6 changes: 6 additions & 0 deletions src/components/Attachments/AttachmentView/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,8 @@ const propTypes = {
transactionID: PropTypes.string,

isHovered: PropTypes.bool,

optionalVideoDuration: PropTypes.number,
};

const defaultProps = {
Expand All @@ -70,6 +72,7 @@ const defaultProps = {
maybeIcon: false,
transactionID: '',
isHovered: false,
optionalVideoDuration: 0,
};

function AttachmentView({
Expand All @@ -93,6 +96,7 @@ function AttachmentView({
fallbackSource,
transaction,
isHovered,
optionalVideoDuration,
}) {
const {updateCurrentlyPlayingURL} = usePlaybackContext();
const theme = useTheme();
Expand Down Expand Up @@ -206,11 +210,13 @@ function AttachmentView({
}

if (isVideo || (file && Str.isVideo(file.name))) {
console.log({optionalVideoDuration});

Check failure on line 213 in src/components/Attachments/AttachmentView/index.js

View workflow job for this annotation

GitHub Actions / lint

Unexpected console statement
return (
<AttachmentViewVideo
source={source}
shouldUseSharedVideoElement={isUsedInCarousel}
isHovered={isHovered}
videoDuration={optionalVideoDuration}
/>
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ function VideoRenderer(props) {
const duration = Number(htmlAttribs[CONST.ATTACHMENT_DURATION_ATTRIBUTE]);
const activeRoute = Navigation.getActiveRoute();
const {reportID} = parseReportRouteParams(activeRoute);
console.log({duration});

Check failure on line 26 in src/components/HTMLEngineProvider/HTMLRenderers/VideoRenderer.js

View workflow job for this annotation

GitHub Actions / lint

Unexpected console statement
return (
<VideoPlayerPreview
videoUrl={sourceURL}
Expand Down
4 changes: 2 additions & 2 deletions src/components/VideoPlayer/BaseVideoPlayer.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ function BaseVideoPlayer({
const styles = useThemeStyles();
const {isSmallScreenWidth} = useWindowDimensions();
const {playVideo, currentlyPlayingURL, updateSharedElements, sharedElement, originalParent, shareVideoPlayerElements, currentVideoPlayerRef} = usePlaybackContext();
const [duration, setDuration] = useState(videoDuration);
const [duration, setDuration] = useState(videoDuration * 1000);
const [position, setPosition] = useState(0);
const [isPlaying, setIsPlaying] = useState(false);
const [isLoading, setIsLoading] = useState(true);
Expand All @@ -61,7 +61,7 @@ function BaseVideoPlayer({
const isVideoPlaying = e.isPlaying || false;
setIsPlaying(isVideoPlaying);
setIsLoading(Number.isNaN(e.durationMillis)); // when video is ready to display duration is not NaN
setDuration(e.durationMillis || 0);
setDuration(e.durationMillis || videoDuration * 1000);
setPosition(e.positionMillis || 0);
}, []);

Check warning on line 66 in src/components/VideoPlayer/BaseVideoPlayer.js

View workflow job for this annotation

GitHub Actions / lint

React Hook useCallback has a missing dependency: 'videoDuration'. Either include it or remove the dependency array. If 'setDuration' needs the current value of 'videoDuration', you can also switch to useReducer instead of useState and read 'videoDuration' in the reducer

Expand Down

0 comments on commit 699e302

Please sign in to comment.