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

feat: live_video_preview #2415

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

Conversation

drumusician
Copy link
Contributor

@drumusician drumusician commented Jan 24, 2023

Live Video Preview

For a project I am working on we needed a live video preview. After trying to implement it in our project it was far easier and made more sense to do that here.

So this PR adds:

<.live_video_preview entry={@entry} />
  • also added a basic test for both this new component as well as the existing live_img_preview

@chrismccord
Copy link
Member

Since live_img_preview can be used for this purpose and is the exact same code wise, I'd prefer to wait until we have video-specific features that make sense. Thanks!

@drumusician
Copy link
Contributor Author

hey Chris, thanks for looking at this. How would I use live_img_preview to generate a live video preview?

@drumusician
Copy link
Contributor Author

To have an idea, this is what I'm aiming for:

Scherm­afbeelding 2023-01-24 om 22 32 21

@chrismccord chrismccord reopened this Jan 24, 2023
@chrismccord
Copy link
Member

Sorry, I thought the code comment was referring to both js and server, so the difference its he video tag. I know the some video file types for me work just fine with live_img_preview, but it may not be as comprehensive. Let me explore this one some more. Thanks!

@drumusician
Copy link
Contributor Author

ok, yeah the Hook can be removed indeed. I'll push a new commit removing that part

@drumusician
Copy link
Contributor Author

ok, another thought I am having concerning this is maybe we just need a <.live_preview /> or <.live_asset_preview /> component that is a bit smarter and can decide what to use for the preview based on the mimetype.

What do you think about that idea?

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