search

convos-chat / linkembedder

Embed / expand oEmbed resources and other URL / links
6 stars 2 forks source link

Fixed direct video links not rendering correctly. #26

Closed brunoramoslu closed 3 years ago

brunoramoslu commented 3 years ago

Fixed direct video links not rendering correctly. Added webp and webm formats.

Direct video links[1][2] were not being rendered correctly and I were always showing a message: Your browser does not support the video tag.

I did some changes to correctly create a <div><video><source> structure so that the video is rendered correctly. Still need to update the tests in basic.t and clean up the %for in the video.html.ep template. And the video is being rendered at 100%, so that needs some cleanup too (review class and width/height).

Could you please take a look already to see if you understand what I did? Maybe provide some guidance/comments?

[1] https://www.learningcontainer.com/wp-content/uploads/2020/05/sample-mp4-file.mp4 [2] https://dl8.webmfiles.org/big-buck-bunny_trailer.webm

brunoramoslu commented 3 years ago

I did a round of changes and fixed the basic.t tests. I'm checking it it was me who broke the other test... I'm having failures when using TEST_ONLINE=1 on some others tests.

brunoramoslu commented 3 years ago

I think I did most of what I was planning. I'm glad to accommodate any other suggestions. If there's nothing else you would like changed related to this, feel free to accept it.

I was wondering if there's something else needed to better integrate the changes in convos... but I do not know the code base well enough to give an informed opinion about that.

jhthorsen commented 3 years ago

I was wondering if there's something else needed to better integrate the changes in convos... but I do not know the code base well enough to give an informed opinion about that.

Not sure what that would be, but please let me know if you have any specific ideas.