search

orchidhq / Orchid

Build and deploy beautiful documentation sites that grow with you
https://orchid.run
GNU General Public License v3.0
513 stars 53 forks source link

Pixel to ratio in OrchidWriters youtube #331

Closed DanySK closed 4 years ago

DanySK commented 4 years ago

Hi, I developed a tag for embedding YouTube videos into Orchid sites, based on the existing one in Writers, with a difference: rather than asking the user for the size in pixel, it occupies the horizontal space available, and the user can specify the form factor in case he is dealing e.g. with vertical or 4:3 videos.

the tag can be used with:

{% ytvideo id='606ObQwQuaE' start='0:5' aspectRatio='18:10' %}

I believe it could replace the existing tag in writers. Would you accept a PR? Alternatively, I can release it as a separate artifact (I probably will anyways as I need it shortly, but I could well drop it if it gets included in an official Orchid module).

Let me know

cjbrooks12 commented 4 years ago

That would be a great addition, I'd gladly accept a PR to update the current tag! My only suggestion would be to not remove the pixel-sizing option, so that it remains backward-compatible. So then you'd have to check if it's sized using the aspect ratio or with pixel sizing, and maybe throw an error if both are used. You can override the onRender function in the TemplateTag class and perform the validation check there.

If you could get a PR submitted in the next few days, I'll include it in 0.17.7 which I should be releasing sometime this weekend. Please fork from the features/0.17.7 branch in that case. Otherwise, if you can't get to it that soon, branch off of dev and it will be available in 0.18.0 towards the end of November.

cjbrooks12 commented 4 years ago

I've just released 0.17.7, so please branch off dev when you start working on this PR. Thanks!

DanySK commented 4 years ago

OK. Sorry I had no time to work on it in the weekend

cjbrooks12 commented 4 years ago

No worries, there's no rush, whenever you get time. Just wanted to let you know so you can develop on the right branch 🙂

cjbrooks12 commented 4 years ago

@DanySK I've applied your changes to the YouTube tag here, it's looking great! I'll have it released in the next couple of days.

DanySK commented 4 years ago

Wow! Thanks @cjbrooks12, sorry I haven't had time to work on it :)

cjbrooks12 commented 4 years ago

no worries, it was easy enough to copy it from your other project. Now that 0.18 is released, I can finally work on some of these smaller issues!