search

cs50 / video.cs50.net

13 stars 6 forks source link

Widescreen fix #137

Closed coltonoscopy closed 4 years ago

coltonoscopy commented 6 years ago

Temporary solution to allow support for any videos in /mobile, /games, and /web on CDN; avoids parsing JSON for anything by simply string-searching the path. Performs conditional CSS via JavaScript and adds a parameter to the VideoMain and VideoAlt components so they can read the path and do this.

dlloyd09 commented 6 years ago

@coltonoscopy just want to make sure this one's been well-tested before I merge?

kzidane commented 6 years ago

@coltonoscopy @dlloyd09 fwiw, json50 does now add dimensions and duration to video sources. That should be the case for all videos in games, mobile, and web.

cc @dcoffey3296

coltonoscopy commented 6 years ago

@dlloyd09 yep, but feel free to clone and bang on it (via npm install and npm run dev) to see if you caught anything I missed!

@kzidane getting the app to do the resizing based on the JSON would take quite a bit more work (most of the components are asynchronously created and everything is architected very one-way, in a functional style), so this is more of an immediate fix; been talking with @dmalan about potentially adding new features to the player long-term, at which point we can revisit JSON being incorporated and doing asynchronous construction of the components whereby they can programmatically calculate the aspect ratio of the PiP based on raster size.

dmalan commented 6 years ago

@kzidane mind testing and merging when ready? @dlloyd09

kzidane commented 6 years ago

Found a few issues some (or all) of them might be related? https://github.com/cs50/video50/issues?q=is%3Aissue+is%3Aopen+label%3Abug