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

[Demo] Enhance Twitter Data Saver demo #83

Closed
addyosmani opened this issue Oct 30, 2019 · 8 comments
Closed

[Demo] Enhance Twitter Data Saver demo #83

addyosmani opened this issue Oct 30, 2019 · 8 comments

Comments

@addyosmani
Copy link
Collaborator

I believe our Twitter Data Saver demo also included support for embedded videos in Tweets: https://github.com/GoogleChromeLabs/adaptive-loading/blob/master/react-twitter-save-data-loading(hook)/src/components/Tweet/Video.js

If we have time, I would like for us to enhance the current Hooks demo so that the 4th sample tweet is https://twitter.com/mrdoob/status/1131817655134896128. This would allow us to demonstrate what a LQIP/blurred image for a video entry would look like (just an image) when data saver is on and we can then load the video on tap the way Twitter's current experience works.

The video for this tweet is attached 📦

main-video2.mp4.zip

cc @anton-karlovskiy

@anton-karlovskiy
Copy link
Contributor

@addyosmani

Yes, it's clear.

@anton-karlovskiy
Copy link
Contributor

@addyosmani

I've updated the client hint version accordingly since I thought it would take 20 minutes or so.
https://adaptive-loading.web.app/react-twitter-save-data-loading(client-hint)

@anton-karlovskiy
Copy link
Contributor

@addyosmani

FYI: I think the codebase is a bit messy as I just adopted react-tweet project and did not refactor it because I thought it would take quite some time to refactor adequately.

@addyosmani
Copy link
Collaborator Author

No worries. Given this is a demo, I think that is okay. Could we update the tweet text, avatar and username for that entry to match the tweet I linked to as well? Would it be hard to also add this change to the hooks demo?

This was referenced Nov 1, 2019
@anton-karlovskiy
Copy link
Contributor

No worries. Given this is a demo, I think that is okay. Could we update the tweet text, avatar and username for that entry to match the tweet I linked to as well? Would it be hard to also add this change to the hooks demo?

I think it's not so difficult. After adding, we might have to adjust the view on desktop and mobile.

@anton-karlovskiy
Copy link
Contributor

anton-karlovskiy commented Nov 10, 2019

@addyosmani

The look and feel of the video tweet is updated with copy of the original appearance.
Please review this.
Capture

Capture

The links are also forwarded to the relevant original sources like author profile, orignal tweet and glitch demo.

@anton-karlovskiy
Copy link
Contributor

anton-karlovskiy commented Nov 10, 2019

@addyosmani

Looks like this project does not quite fit for mobile responsive view, although I tweaked a bit in the past.
This is also mentioned in TODO: of the orignal project.

I've checked that it's okay on Sumsung Galaxy 9 but not guaranteed for all mobile devices.

@addyosmani
Copy link
Collaborator Author

This looks perfect, Anton! Thank you!

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

No branches or pull requests

2 participants