Memmy-App
Feature/video playback
PR Creator Checklist
Ensure you've checked the following before submitting your PR:
- [X] You've discussed making your changes with a member of the dev team per contributing rules in the README
- [X] Your changes are free of any lint errors
- [X] Your changes are free of any typescript errors
- [X] You've tested your changes
Summary
Please provide a summary of what your PR does
Adds initial video playback support
Screenshots
If the UI has been changed, include screenshots. We will not review your PR without screenshots if they are applicable.
It is currently very late at night, I'll do this tomorrow morning
Test Plan
Please documemnt the steps required to test your PR
Test any post you can find with a video link
https://github.com/Memmy-App/memmy/assets/35210888/6136dd78-6f52-4d24-aa7d-936e34ecf67d
https://github.com/Memmy-App/memmy/assets/35210888/6df0b255-fbe6-435b-9b19-ea584300ba27
Sorry for that conflict hell yea...it should just be a matter of fixing imports. Trying to get rid of these babel issues. I was going to fix everything up but I'll wait for now until you finish with this, and a few other branches people are working on. 🙏
So currently there are three major UX issues:
- Video thumbnails are blocking despite being called in an async function (I do not have time to debug this)
- There is no indication of a thumbnail being a video
- The underlying image viewer background gets pulled up (I seem to have a bug with iOS 17 where onFullscreenUpdate isn't called properly so I can't call onRequestOpenOrClose, but I have no idea really)
Sorry for that conflict hell yea...it should just be a matter of fixing imports. Trying to get rid of these babel issues. I was going to fix everything up but I'll wait for now until you finish with this, and a few other branches people are working on. pray
I would have fixed the merge conflicts myself but I've got to leave in 2h and need to pack so unfortunately I'll have to leave that up to you
Sorry for that conflict hell yea...it should just be a matter of fixing imports. Trying to get rid of these babel issues. I was going to fix everything up but I'll wait for now until you finish with this, and a few other branches people are working on. pray
I would have fixed the merge conflicts myself but I've got to leave in 2h and need to pack so unfortunately I'll have to leave that up to you
Yea no problem. I can clean them up myself, just wanted to make sure you were taking a break from the work before I did that.
I'll take a look at those issues you were talking about as well 👍
That's looking pretty good though!
Thanks, I've started work on Media providers so that links from imgur, youtube, redgifs, etc. can also be embeded. I am mentioning redgifs because it's surprisingly the easiest to integrate cus they allow you to generate ephemeral api keys on the fly at runtime. In fact I knew Liftoff supported redgifs so I just used their same method.
@gkansdorf I am about to take off, I'm going to attempt to fix those 3 ux changes while on my flight but I am not sure how easy that is on a crappy intel macbook air. Heck I don't even know if this message will pass through
@gkansdorf I am about to take off, I'm going to attempt to fix those 3 ux changes while on my flight but I am not sure how easy that is on a crappy intel macbook air. Heck I don't even know if this message will pass through
Nvm I can't do this nor do I have time to over the coming couple of weeks. I'll have to revise this in two weeks, either way the groundwork has been laid and anyone is more than free to help with this, especially with the 3 UX problems I identified. @gkasdorf I tried quickly rebasing onto 0.5.6 tag but I quickly realized you've done a lot of changes in that area and I think you know what's best in terms of resolving the merge conflicts
Fixed up the issues. I'll update the PR here in a bit.
I'll save you some time and merge your changes into here, I forgot how much jetlag is a bitch (I travel a lot, yet I still forget how annoying it is) and tonight I find myself with some time on my hands. Or rather boredom of doing nothing practical
Oh and great, according to my git client, there are no merge conflicts. Aye madre
Closes #399, although there are still some major improvements to be made tracked on another branch of mine
Not sure why it's telling me there's conflicts whenever I pull and merge and it's fine...anyway checking it out now.
Edit: Merged in main
I notice that MediaViewer doesn't seem to be displaying anything in the feed. Opening the post does work (since it seems PostContentView still is using ImageViewer as opposed to MediaViewer).
I can look more into why later as well.
Since it looks like (correct me if I'm wrong here?) that you're not directly modifying ImageViewer but instead just creating a separate component for VideoViewer that I could go ahead and make some changes in ImageViewer without affecting this? I have some stuff I want to clean up and fix in there but don't want to get in your way.
Yup, go full ahead. Strange that media isn't showing up in the feed viewer for you. I plan to work on these issues over the coming weeks (or days where possible) and optimize thumbnail loading. As far as maintainability goes I think for the long term ImageViewer and VideoViewer will eventually need to be merged but I think it's a good idea to keep them separate for the meantime because A) different devs are working on two different aspects of media viewing and B) cus there's yet a lot of improvements to be made and experimented with each until desirable results are achieved.