search

gocodebox / lifterlms

LifterLMS, a WordPress LMS Solution: Easily create, sell, and protect engaging online courses.
https://lifterlms.com
GNU General Public License v3.0
184 stars 135 forks source link

Does LifterLMS incorrectly use the `[video]` shortcode to power video embeds in courses and lessons? #833

Closed thomasplevy closed 5 months ago

thomasplevy commented 5 years ago

I've noticed themes (like Astra) are filtering the HTML return of the [embed] shortcode to make video embeds automatically responsive.

They are not filtering [video] embeds (should they, I'm not sure, I need to do more research).

However, in digging into this to determine why LifterLMS videos in the course & lesson fields aren't automatically responsive on Astra I found this in the WordPress core:

  • This implements the functionality of the Video Shortcode for displaying WordPress mp4s in a post.

Source

So I'm now wondering if we should be using [embed] as opposed to [video] and [audio] in our get_embed() methods on LLMS_Post_Model classes. Source

Partial System Report ``` Theme ------------------------------------------- Name: Astra Version: 1.8.1 Themeuri: https://wpastra.com/ Authoruri: https://wpastra.com/about/ Template: Child Theme: No Llms Support: Yes Plugins ------------------------------------------- Akeeba Backup for WordPress: 3.4.2.2 Astra Pro: 1.8.1 Astra Widgets: 1.1.0 Custom Fonts: 1.0.7 Import / Export Customizer Settings: 1.0.0 Jetpack by WordPress.com: 7.2.1 LifterLMS: 3.30.2 LifterLMS Helper: 3.0.2 LifterLMS WooCommerce: 2.0.9 Loco Translate: 2.2.2 LoginPress - Customizing the WordPress Login: 1.1.21 Mailchimp for WooCommerce: 2.1.15 Mollie Payments for WooCommerce: 5.1.6 Really Simple SSL: 3.1.5 Ultimate Addons for Gutenberg: 1.12.5 Uncanny LearnDash Toolkit: 3.0.2 WooCommerce: 3.5.8 WooCommerce PDF Invoices & Packing Slips: 2.2.11 WooCommerce Services: 1.19.0 WP-Optimize: 2.3.3 WPForms Lite: 1.5.2.2 ```

Problem site was using Vimeo videos

thomasplevy commented 5 years ago

@Nikschavan, @sujaypawar or anyone at Brainstorm if you'd be willing to weigh in with any thoughts or opinions on this. As I noticed this as a result of a support request from a user using Astra with LifterLMS I'm doing my testing and thinking against Astra. If anyone over there has any opinions on whether or not we're misusing the core shortcode I'd love to hear them. Thanks as always!

thomasplevy commented 5 years ago

@actual-saurabh If you can read this over and give me your 2cents I'd appreciate it also

Nikschavan commented 5 years ago

LifterLMS videos in the course & lesson fields aren't automatically responsive

When I am testing this, At least, Youtube/Vimeo/wistia videos added in the featured video setting of a course or a lesson appear responsive and take up complete content width. (screenshot)

Are you referring to any particular video format or videos added from some other location?

thomasplevy commented 5 years ago

@Nikschavan

Thanks for weighing in.

The video in question was a Vimeo embed. I traced it back (albeit it quickly) to what I thought was this issue (using [video] not [embed] but perhaps theres something else going on on this particular site.

I'll setup a more specific test case to work further. In the end I don't think this is an Astra issue at all but I was just curious if y'all had any thoughts on the usage of one shortcode over the other and if there's a semantic issue here.

I've added the plugin and theme info from our system report to the issue and I'll setup a better test case in the future.

Thanks again!