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: video thumbnail support and translations #38

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

Conversation

iberflow
Copy link

Summary

I've added thumbnail support which enables us to use thumbnails for videos. Since temp upload does not use Spatie's media library, therefore does not generate thumbnails, I've replaced the thumb with a "play" icon for video uploads.

Changes are necessary for my application and it also closes #35, #37.

I've not added new test cases, only fixed the current ones. I'll wait for feedback on whether you want this merged, but my use case requires these changes, therefore I'll keep my fork up.

Screenshot 2023-03-22 at 02 26 21

Screenshot 2023-03-22 at 02 25 46

Screenshot 2023-03-22 at 02 25 19

Checklist

  • I checked my (code) changes for obvious issues, debug statements and commented code
  • Documentation (if necessary)
  • [~] Tests (if necessary)
  • Ready to be merged

@iberflow iberflow changed the title Feature/video support Video thumbnail support and translations Mar 22, 2023
@iberflow iberflow force-pushed the feature/video-support branch from 583aca2 to 2738d13 Compare March 22, 2023 00:40
@ItsANameToo ItsANameToo changed the title Video thumbnail support and translations feat: video thumbnail support and translations Mar 23, 2023
@ItsANameToo
Copy link
Contributor

thank you @ignasbernotas , I'll have a look at this PR soon!

@iberflow
Copy link
Author

iberflow commented May 4, 2023

hey @ItsANameToo do you need a hand with this? I'm happy to prepare this for a merge, just need a little feedback from you :)

@ItsANameToo
Copy link
Contributor

hey @ignasbernotas sorry for the delay, it's been busy and this hasn't been a high enough priority to get to it seems. The PR in itself looks fine, so if you could prepare some testcases to go along with it it's something we're willing to merge.

You'll have to run the tests locally as the workflows on the PR require a token that's not available to outside collaborators sadly

@scramatte
Copy link

Hello,

Any chance to get this PR merged to master?

Regards

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

Successfully merging this pull request may close these issues.

[Feature Request] - Ability to Upload Videos
4 participants