Automattic / sensei

Sensei LMS - Online Courses, Quizzes, & Learning
https://senseilms.com
GNU General Public License v2.0
540 stars 198 forks source link

Course settings about “Course image” not having any effect. #5291

Open aaronfc opened 2 years ago

aaronfc commented 2 years ago

Reported by Gonzalo here: p1654590027797169-slack-C03BXN7UQ5V

Reproduce:

Extra info:

mikeyarce commented 2 years ago

I've been looking into this and there are currently a few issues I found with featured image options:

Block Themes Course Lesson
Single Page Show ✅ (but not custom size) ✅ (but double image)
Archive Show*
Single Page Hide
Archive Hide**
PHP Template Themes Course Lesson
Single Page Show ✅ (but not custom size) ✅ (but double image)
Archive Show*
Single Page Hide
Archive Hide**

*The Archive featured image display option was moved to the block metabox, so the Settings option doesn't work. ** Currently there is no option to show/hide Lesson images on the Course Page.

Example double featured image (blockbase theme):

Screen Shot 2022-07-15 at 2 10 39 PM

The problem is that Sensei is trying to output inside a template using a hook, but Sensei sometimes uses PHP templates and other times uses Blocks. In the Lesson example, the reason there's a double output is that one is the theme outputting it in the header and the second one is Sensei outputting it in the sensei_single_lesson_content_inside_before hook.

Are we trying to support both php template use and block use?

My proposal:

  1. We get rid of outputting images in PHP templates and just rely on themes to output images appropriately https://github.com/Automattic/sensei/blob/4851abbd48ffd5bacd1139682d24764d18bb1fa1/includes/class-sensei-lesson.php#L3682
  2. We add some hooks so that the actual featured_image either shows/hides depending on the site settings. We would hook into post_thumbnail_html and post_thumbnail_size to show/hide and change the image sizes.

@aaronfc what do you think?