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.

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

✨ Allow user to disable video preview #406

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

pajowu
Copy link
Member

@pajowu pajowu commented Dec 9, 2023

No description provided.

Base automatically changed from phlmn/basic-video-preview to main December 10, 2023 00:01
@pajowu pajowu force-pushed the pajowu/enable_video_prev branch from 82a1875 to ed407ad Compare December 10, 2023 00:21
@pajowu pajowu enabled auto-merge December 10, 2023 00:22
@pajowu pajowu requested review from phlmn, rroohhh and anuejn December 10, 2023 00:22
frontend/src/editor/player.tsx Outdated Show resolved Hide resolved
hasVideo: videoFiles.length > 0,
};
}, [data?.media_files]);
const [_showVideo, setShowVideo] = useState(true);
Copy link
Member

Choose a reason for hiding this comment

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

Let's please name the state variable consistent with the setter :D

Copy link
Member

Choose a reason for hiding this comment

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

Nevertheless approved already :)

@phlmn phlmn disabled auto-merge December 10, 2023 11:29
@phlmn phlmn self-requested a review December 10, 2023 11:31
Copy link
Member

@phlmn phlmn left a comment

Choose a reason for hiding this comment

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

Hm... just tested this and I get two competing audio streams playing at the same time, when disabling the video...

@phlmn
Copy link
Member

phlmn commented Dec 10, 2023

Also the original media is not used as fallback on videos, since it does not have the video tag. But I don't think the fallback is too important anyway.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants