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

Video support. #40

Merged
merged 12 commits into from
Mar 4, 2024
Merged

Video support. #40

merged 12 commits into from
Mar 4, 2024

Conversation

Ketchupchh
Copy link
Contributor

@Ketchupchh Ketchupchh commented Jan 6, 2024

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

Copy link

vercel bot commented Jan 6, 2024

Someone is attempting to deploy a commit to a Personal Account owned by @ccrsxx on Vercel.

@ccrsxx first needs to authorize it.

@DaniKasti
Copy link

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

@Ketchupchh
Copy link
Contributor Author

Thank you. I will fix this.

@ccrsxx
Copy link
Owner

ccrsxx commented Jan 12, 2024

I will review it next week @Ketchupchh hehe.

I'll be done with my final semester at uni this week.

Copy link

vercel bot commented Jan 17, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
twitter-clone ✅ Ready (Inspect) Visit Preview 💬 Add feedback Mar 3, 2024 4:35pm

Copy link
Owner

@ccrsxx ccrsxx left a 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.

@Ketchupchh
Copy link
Contributor Author

Awesome, thanks!

@ccrsxx
Copy link
Owner

ccrsxx commented Feb 14, 2024

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.

@Ketchupchh
Copy link
Contributor Author

Take your time man. Life comes first.

@ccrsxx
Copy link
Owner

ccrsxx commented Feb 18, 2024

@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.

@Ketchupchh
Copy link
Contributor Author

@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.

@ccrsxx
Copy link
Owner

ccrsxx commented Feb 19, 2024

Damn, I forgot to test it on mobile. I'll take a look later, bruh I thought I could merge it yesterday.

@ccrsxx
Copy link
Owner

ccrsxx commented Feb 27, 2024

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.

@Ketchupchh
Copy link
Contributor Author

Yeah, we should probably add a button to keep the Twitter theme quality.

@ccrsxx
Copy link
Owner

ccrsxx commented Mar 3, 2024

@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?

@Ketchupchh
Copy link
Contributor Author

The video modal just infinite loads for me :(. Maybe it's just a problem on my end? :p

@ccrsxx
Copy link
Owner

ccrsxx commented Mar 3, 2024

let me check using another account to make sure it's not a perms issue.

@Ketchupchh
Copy link
Contributor Author

I doubt it? My own video also causes the modal to infinite load.

@ccrsxx
Copy link
Owner

ccrsxx commented Mar 3, 2024

Hmm works fine on my end, even using another account that's not an admin.

image

Can you check the web console, and see if there's any error there?

@Ketchupchh
Copy link
Contributor Author

Sorry, I should've mentioned it infinite loads on mobile. Desktop works just fine.

@ccrsxx
Copy link
Owner

ccrsxx commented Mar 3, 2024

Works fine on my phone.

Screenshot_20240303_234939_Samsung Internet

Maybe there's some ad block/extension that blocks the video? Have you tried it on another browser, to see if it works?

@Ketchupchh
Copy link
Contributor Author

Yeah, I've tried it on Safari and OperaGX. Has to be something on my end since for some reason on mobile your video fails to load all together but on desktop it loads fine except the video doesn't show just the audio. imageimage

@ccrsxx
Copy link
Owner

ccrsxx commented Mar 3, 2024

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.

@Ketchupchh
Copy link
Contributor Author

This is all I get. Probably is just an issue on my end. image

@ccrsxx
Copy link
Owner

ccrsxx commented Mar 3, 2024

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.

@Ketchupchh
Copy link
Contributor Author

Okay yeah, Chrome makes everything work fine lol.

@ccrsxx
Copy link
Owner

ccrsxx commented Mar 4, 2024

Nice, will close this PR tonight.

@ccrsxx ccrsxx merged commit 6722c14 into ccrsxx:main Mar 4, 2024
4 checks passed
Altair59 pushed a commit to Altair59/cyclone-v2 that referenced this pull request Mar 6, 2024
* 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]>
h-lunah pushed a commit to h-lunah/OpenTwitter that referenced this pull request Oct 20, 2024
* 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]>
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.

3 participants