-
Notifications
You must be signed in to change notification settings - Fork 183
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
Video support. #40
Video support. #40
Conversation
Someone is attempting to deploy a commit to a Personal Account owned by @ccrsxx on Vercel. @ccrsxx first needs to authorize it. |
I believe there is a bug with the video audio. If you click on the video there are two audio's playing - one in the background and one in the clicked on version. Not a huge deal. Just something I noticed. @Ketchupchh |
Thank you. I will fix this. |
I will review it next week @Ketchupchh hehe. I'll be done with my final semester at uni this week. |
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall finished the review. I will check if there's any bug before merging it later.
Awesome, thanks! |
Sorry, I'm really busy with work that I couldn't merge this earlier. But I'll be sure to merge this week tho (if there's no problem), all that's left is to test things out. |
Take your time man. Life comes first. |
@Ketchupchh Can you check the preview? Just wanted to see if everything works. It works fine on my end. If there's no problem I can merge it. |
@ccrsxx Everything works fine on desktop. On mobile I was able to load my video but not yours and the image modal never loaded the video either. |
Damn, I forgot to test it on mobile. I'll take a look later, bruh I thought I could merge it yesterday. |
Btw I just checked again, it's just the modal that's not coming out on mobile. I'm able to see your video tho. Hmm, should we add a button on the video on mobile to open the modal or keep it this way? I see that Twitter doesn't open the modal when you click on a video. |
Yeah, we should probably add a button to keep the Twitter theme quality. |
@Ketchupchh I added a button to open the video modal on mobile, but not on desktop since you open the modal by clicking the video there. I think it's okay to merge it now, can you check it hehe? |
The video modal just infinite loads for me :(. Maybe it's just a problem on my end? :p |
let me check using another account to make sure it's not a perms issue. |
I doubt it? My own video also causes the modal to infinite load. |
Sorry, I should've mentioned it infinite loads on mobile. Desktop works just fine. |
Check the web console on the desktop, is there any errors there? I'll ask my friends to check the video tomorrow, if it's working, then it must be from your end. |
I think it's a Safari issue, I remember something broke from my website on Safari but not on other browsers. Can you try using the Chrome browser on your desktop? Don't try it on your iPhone, because all the browsers there are just a custom skin for Safari lol. |
Okay yeah, Chrome makes everything work fine lol. |
Nice, will close this PR tonight. |
* video now mutes when opening video modal * username bug fix & images upload by id * refactor: reuse logic when checking if username exists * style: minor nit changes spaces and newline * feat: improve logic when handling uploading media * feat: improve logic when extracting image and video data * feat: remove alt check when uploading media * feat: overall finished pr * feat: make tweet card non draggable to allow sliding on video progress * feat: add button on video tweet to open modal on mobile --------- Co-authored-by: ccrsxx <[email protected]>
* video now mutes when opening video modal * username bug fix & images upload by id * refactor: reuse logic when checking if username exists * style: minor nit changes spaces and newline * feat: improve logic when handling uploading media * feat: improve logic when extracting image and video data * feat: remove alt check when uploading media * feat: overall finished pr * feat: make tweet card non draggable to allow sliding on video progress * feat: add button on video tweet to open modal on mobile --------- Co-authored-by: ccrsxx <[email protected]>
I added video support and a very simple fix (from my testing) that prevents usernames from being stolen.
Demo: https://twitter-clone-swart-three.vercel.app/
Obviously, the code for video support is in here.
I left the original code intact in case I broke something :P