[wp-trac] [WordPress Trac] #34561: Abstract some embed template code into functions

[WordPress Trac]

#34561: Abstract some embed template code into functions
-------------------------------------+---------------------------
 Reporter:  johnbillion              |       Owner:  DrewAPicture
     Type:  enhancement              |      Status:  reviewing
 Priority:  normal                   |   Milestone:  4.5
Component:  Embeds                   |     Version:  4.4
 Severity:  normal                   |  Resolution:
 Keywords:  has-patch needs-testing  |     Focuses:
-------------------------------------+---------------------------

Comment (by imath):

 Hi @swissspidy

 Sorry it took me so long to reply and thanks a lot for the ping :)

 Well WP Idea Stream is filtering `embed_template` and is using its own
 template not necessarly because of "not enough hooks" inside the `embed-
 template.php` file, but because a user is not a post.

 As the embed content is only displayed if `have_post()` and as i had no
 way to filter the "not found" embed heading and excerpt, filtering `embed-
 template.php` seemed the only way for me.

 I guess your patch is improving a bit this for the "not found" excerpt,
 but i'd still have to hide the "not found" heading using css or
 javascript. But, to be honest, on the other hand i don't see a reason to
 change something in my plugin because today it's avoiding a post query
 loop for a post that is not existing anyway :)

 imho, i'm not sure having a lot of hooks is the best road, i saw you're
 adding hooks before and after almost each tag which is nice. I'm a bit
 used to it with BuddyPress templates :)

 I think the problem with this is: i saw you are planing to use
 `get_query_template()` to eventually get the embed template within the
 theme. In this case having too much hooks can be risky because it gives
 the illusion to a plugin author he can safely play at a place that won't
 be there at 100% if the theme is removing some of these hooks within its
 template. And i've just looked at some theme's template, it looks like
 it's not a common practice to include hooks inside theme templates. Proof
 is: the only `do_action` available in twentysixteen is `do_action(
 'twentysixteen_credits' );`;) So i guess adding hooks is great as soon as
 they are inside template tag functions.

 I saw that you were also loading `locale_stylesheet` inside the
 `embed_header`, there might be a good reason i don't see, but it seems a
 bit weird to me to load a complete theme stylesheet for a few parts to
 style.

 More globally:

 I'm very interested about the `get_query_template()` road and your .4
 patch on #34278. imho this road means you need to do things the way theme
 authors are used to. It looks a possible way to achieve this is to:

 1/ Add new templates inside `wp-includes/theme-compat` :
 header-embed.php where you could put the specific header for embed
 content.
 footer-embed.php where you could put the specific footer for embed
 content.
 content-embed.php where you could put embed content

 2/ Edit the embed-template.php to use functions like this

 `get_header( 'embed' )` which would load the header-embed.php template of
 the theme or fallback in `wp-includes/theme-compat`
 `get_footer( 'embed' )` which would load the footer-embed.php template of
 the theme or fallback in `wp-includes/theme-compat`

 3/ Rename `print_embed_thumbnail()` to something like
 `the_post_embed_thumbnail()` so that it sounds like a template tag :) and
 leave theme choose whether to use or not the thumbnail into their embed
 content.

 4/ Create a new template tag to output the embed content, something like
 this
 {{{
 function get_embed_template_part() {
         // First try to use the content-embed.php template part provided
 by the theme
         if ( '' !== get_template_part( 'content', 'embed' ) ) {
                 return;
         }

         // Nothing found, let's use the default output
         load_template( ABSPATH . WPINC . '/theme-compat/content-embed.php'
 );
 }
 }}}

 This way embed-template.php could only take 3 lines and would become a lot
 more customizable :)
 {{{
 <?php get_header( 'embed' );
 get_embed_template_part();
 get_footer( 'embed' );
 }}}

 If you think it's a good idea, i can try to suggest a patch about it, but
 i guess it's requiring #34278 to be committed first ;)

--
Ticket URL: <https://core.trac.wordpress.org/ticket/34561#comment:22>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform