Epic: Unified Embed

[ludwiczakpawel]

• Related pod issue
• Working doc

[github-actions[bot]]

Thanks for the issue, we will take it into consideration! Our team of engineers is busy working on many types of features, please give us time to get back to you.

Feature requests that require more discussion may be closed. Read more about our feature request process on forem.dev.

To our amazing contributors: issues labeled type: bug are always up for grabs, but for feature requests, please wait until we add a ready for dev before starting to work on it.

To claim an issue to work on, please leave a comment. If you've claimed the issue and need help, please ping @forem/oss. The OSS Community Manager or the engineers on OSS rotation will follow up.

For full info on how to contribute, please check out our contributors guide.

[ludwiczakpawel]

Hey, @Posandu, I appreciate you looking into this issue, however this is something we will likely take care of internally as Forem Team (this document is indeed internal). I should have added "Forem team" label earlier which would likely prevent this confusion – I apologize for that.

[msarit]

My current approach with this Epic is to implement {% embed url %} for all Liquid Tags, BUT leave the original {% liq-tag-name input %} syntax in place.

For example, for the YoutubeTag, I am implementing {% embed https://www.youtube.com/embed/fhH5xX_yW6U %} but leaving the original {% youtbe fhH5xX_yW6U %} in place.

In the near future, pod-CE will decide how to deprecate the original LiquidTag implementations.

= = = = = = = = =

LiquidTags with a working Unified Embed:

• [X] Asciinema
• [X] Blogcast
• [X] Codepen
• [X] Comment
• [x] Codesandbox
• [X] DotNetFiddle
• [x] Forem Listing
• [x] Forem Podcast
• [x] Forem Tag
• [x] Forem User Profile
• [X] Forem Link
• [X] Forem Organization
• [X] Github Gist
• [x] GitHub Issue
• [x] GitHub README
• [X] Glitch
• [X] Instagram
• [X] JSFiddle
• [X] Jsitor
• [X] Kotlin
• [X] Medium
• [X] NextTech
• [X] Reddit
• [X] Replit
• [x] Slideshare
• [x] Speakerdeck
• [X] Soundcloud
• [X] Spotify
• [x] Stackblitz
• [x] Stackery
• [X] StackExchange/StackOverflow
• [X] Twitch
• [X] Twitter
• [X] TwitterTimeline
• [X] Wikipedia
• [X] Vimeo
• [X] YouTube

LiquidTags being left as-is, and why:

• [X] UserSubscription - renders a "Subscribe to this Forem" box and button; can only be created by admins and super_admins.
• [X] Details creates the <details> HTML element
• [X] RunKit => leaving as-is; does not take URL as input; renders interactive javascript playgrounds.
• [X] Katex => leaving as-is; does not take URL as input; renders mathematical computations visually.
• [X] Forem Poll - the Poll feature (which this tag renders on the page) was never widely released. On DEV, there are 4 articles with 6 polls between them. According to Product, the poll feature will be built out later, which may necessitate re-building the PollTag as well. Documentation-wise, the PollTag will be left out.
• [X] NullTag - used to prevent certain words from being used in rendering LiquidTags. These words are:

  assign break capture case cycle decrement echo for if ifchanged include increment render tablerow unless

LiquidTags without a Unified Embed, and reasons why:

• [X] GitPitch (has been shut down permanently)
• [X] Parler (parler keyword returns {"message": "Internal server error"}; I think the service is not active anymore)

[ludwiczakpawel]

I'm suuuuper excited and pumped about this project! So glad to see you working on this and can't wait for this to be shipped! 🤩 🤩 🤩

Couple questions/notes:

1. Leaving the original liquid tag format for now is definitely a good approach. Will be easier for users to transition to new format while we can still have some time to deprecate the old one.
2. Looking at your list above, what does it mean when you say that some liquid tag are "without a unified embed" and some are "with a working unified embed"?
3. Is this something you're thinking about by any chance? Not saying this needs to be implemented right away but having some sort of strategy here would be very helpful in our future work, so it's probably worth looking into while we're on this task. Also cc'ing @jeremyf here.
4. Correct me if I'm wrong but weren't we aiming for { http://example.com } format instead of {% embed http://example.com %} (so no % and no embed keyword)? At least that's what we spec'd.
5. You don't mention that generic embeds idea for the rest of the URLs (that we currently not support) - is this still something on the radar? I'm sure it is, just making sure we're all on the same page :)))

[jeremyf]

Correct me if I'm wrong but weren't we aiming for { http://example.com } format instead of {% embed http://example.com %} (so no % and no embed keyword)? At least that's what we spec'd.

That's a good question. In the liquid parsing the implementation of the embed tag follows the current other tag implementations. So, while it might be possible to get to { embed } I suspect that we're going to have to store {% embed %} (but that's based on now decade old memories of last working with Liquid).

[msarit]

Hey @ludwiczakpawel thanks for your comments and questions

Correct me if I'm wrong but weren't we aiming for { http://example.com } format instead of {% embed http://example.com %} (so no % and no embed keyword)? At least that's what we spec'd.

Hmmm I missed this in the specifications file. Hmmm I would need to discuss with @jeremyf about the { url } approach. My current approach is to unify all tags to be of the form {% embed url %}. Perhaps after this incremental change, we could explore not needing the embed keyword.

This project is a huge one, and will require a gradual, incremental rollout, as well as ensuring that existing tags are not broken. Jeremy is working with me, so we shall certainly fulfill all spec requirements.

[ludwiczakpawel]

All good, just wanted to rise my concerns/questions before it's too late :)

(there was a gif but it as driving me nuts)

[jeremyf]

You don't mention that generic embeds idea for the rest of the URLs (that we currently not support) - is this still something on the radar? I'm sure it is, just making sure we're all on the same page :)))

I'm glad your helping us keep this salient. And I think I'm going to spend some time articulating the larger roll out plan. I know we have a google doc, but we're heading into a checklist kind of effort.

What Arit's doing is to shore up the interface, so we can reduce immediate overhead. Then in quick order, look into using OpenGraph (or Twitter Cards or what have you) to see about doing a more generalized approach for embeds.

[ludwiczakpawel]

@jeremyf great. I just wasn't aware of that (sorry, I might have missed a doc or something). So thank you for quick explanation!

[msarit]

I'm closing this issue as the work for this iteration of streamlining rich-content embeds has been completed. 🎉

[ludwiczakpawel]

What about Generic Embeds? It's been part of this issue and I don't want it to be forgotten.

[amywtlin]

@ludwiczakpawel Thanks so much for catching this and yes this needs to be done.

[justin-schroeder]

This is a great upgrade! Thanks for everyone’s hard work (especially @msarit). I'm not super familiar with the forem -> dev.to pipeline, but I'm pretty sure dev.to articles dont support this yet even though the docs indicate they do. Is that possible? I'm getting errors over there trying to do things like this:

{% embed https://stackblitz.com/edit/intro-example-a?embed=1&file=src/App.vue&hideExplorer=1&hideNavigation=1&view=preview %}

🙏

[ludwiczakpawel]

@justin-schroeder hmm, interesting - that very URL does work for me on dev.to... Could you try to clear your cache and if it's still broken --> report an issue?