I’ve gotten myself into a bit of a problem.
A couple of months ago, we released a new feature called Site Logo on WordPress.com, that allows you to set a logo for your site and have it persist between theme changes. It went over well, and it was decided to roll it into Jetpack for .org users. However, part of that Jetpack integration involved prefixing the template tags used by themes, leaving me with inconsistent function names once we merge Jetpack back to WordPress.com (and for those using the .org Github plugin). It also leaves our premium theme sellers wondering what template tags they should be using moving forward.
I was discussing how to handle the transition with George Stephanis, the Jetpack team lead, and he suggested something I hadn’t considered: have themes just add a hook rather than using template tags when adding features. So rather than theme devs outputting a site logo by adding:
<?php if ( function_exists( 'jetpack_the_site_logo' ) ) jetpack_the_site_logo(); ?>
it could just be:
<?php do_action( 'jetpack_site_logo' ); ?>
I admit it weirds me out: I want to use template tags, the same way I do to output a post title or a featured image, and I imagine people hanging all sorts of strange stuff from it, because, well, it’s a hook. The advantages are clear, though: code behind that hook can be changed and evolve with little concern for theme compatibility, no need for the function_exists() dance, and theme devs have an avenue to alter as much or as little as they choose. In fact, any dev can roll their own original implementation, including a totally different Site Logo plugin.
What do you think? Do you prefer the classic use of template tags, or should we move towards hooks for implementing theme features in Jetpack?
ThemeShaper 
18 responses
Hm. How would that work in the case of logo detection? For example, I use a special body class when both logos and site titles are hidden for better layout handling. In this case, it really helps to have the function
has_site_logoavailable (prefixed or non doesn’t matter as much as the boolean being available) for my$classes[]array. What would that look like if no template tags were available?Oh, nothing would change as far as available template tags; you would continue to use
has_site_logo(urgh,jetpack_has_site_logo()) in the same way. In fact, the pull request is only one line: https://github.com/Automattic/jetpack/pull/1221Theme devs will still have changes to make (my apologies to all), but the problem brought up this question in the bigger picture of how features like this should be rolled out.
Lovely. Thanks for the clarification!
The use of hooks is the best way to handle it, definitely.
We use actions quite liberally in the BuddyPress and bbPress default template packs, and as a solution to this kind of problem, and it’s worked quite well – themers gain flexibility, which is the desired end result.
But… I still think it stinks, and heres why: you’re married to the names of those actions forever, in every theme going forward, should you or others want new code to work the old way. If you choose not to include those actions in new themes, you’re alienating an audience of plugins and themes that have purposely opted into supporting your code.
Rather than litter themes with proprietary ‘jetpack_’ actions (and ask themers to do the same) I believe a better long-term solution is having a standard set of template actions: before_the_content, after_the_title, header_logo, and so on. This approach has clearly worked well for WordPress’s internals, and I believe there’s no reason it wouldn’t be equally successful for the externals.
This idea isn’t new; in fact, it’s been years in debate about exactly what these actions should be named and if they should be standardized at all.
I, personally, believe it’s up to prolific themers to champion the cause, to help plugin developers build for flexible extensions to them. Concepts like theme-compatibility could beade much more simple if plugins came with a universal action for outputting the_plugin_content, for example. Without these types of actions in place, plugin authors are forced to do fairly insane juggling to play nicely with any other piece of code that could also be looking to intercept output to some degree.
TL;DR – great idea, needs traction for the community outside of Jetpack.
Apologies for the spelling & grammatical errors. Mobile and autocorrect and all that.
Indeed – there’s this languishing core ticket, as well as the independent work done by the Theme Hook Alliance.
I agree with JJJ 100%. By using a standard action, it also enables support for other plugins that provide a logo upload feature, instead of it being relegated to just Jetpack.
As a theme developer I would create a wrapper function so that a fallback can be created if Jetpack is not installed or the site logo is not set.
In this case the function `jetpack_the_site_logo()` would work fine. Here is an example using `_s`: https://gist.github.com/grappler/063d9988d7b388cba2c1
For the future I think it will depend. I don’t think a plugin specific action is best. I think it would be best to support the Theme Hook Alliance. A number of themes have started to support it. At the moment a `tha_the_site_logo` action is missing but perhaps could be discussed and added.
In general, that’s a solid approach, but in the case of Site Logo, there are a couple of things to mention here.
jetpack_the_site_logo()should always run regardless of whether a logo is set or not. This is so that a placeholder can be output instead when in the Customizer, which is necessary for the live preview to work when setting a logo. No markup will be output on the front-end however, if there is no logo set.Also, the site logo should be considered independent of the site title and tagline; one should not affect the other. Users are given the choice in the Customizer of hiding the title and tagline, so they will keep the final control of what combination, if any, they want of logo, title, and tagline. Implementing it as one of the other is breaking core functionality.
Ok, I changed the code to fallback to the normal header image which is normally used for the logos but that could be another plugin. https://gist.github.com/grappler/063d9988d7b388cba2c1
As @grapplerulrich mentioned… we need a way to provide a fallback if Jetpack is not in use. I am not sure the hook approach provides us with a simple way to do that?
That’s the beauty of the action method; if nothing is on that hook (say, because Jetpack isn’t installed), then it simply does nothing (with no errors). No need to provide a fallback or
function_exists()check.It would be a bit unreasonable to not have a fallback logo option if Jetpack is not installed.
Even if a fallback was added via the hook, one would still need to check if Jetpack is installed.
add_action( 'jetpack_the_site_logo', '_s_the_site_logo' );If the theme author wanted to use an action he could just create his own hook and add a single line to the theme.
add_action( '_s_the_site_logo', 'jetpack_the_site_logo' );“Theme devs will still have changes to make”.. Could you specify what changes exactly should be made for the theme? Currently I’m using this: https://github.com/Automattic/jetpack/issues/1204#issuecomment-59667689
Yes, that solution works. Moving forward, template tags will be prefixed, and the standalone plugin will be retired once the official Jetpack release with Site Logo is out.
Thanks for the feedback everyone!
We’ve decided that without core support for a standardized set of theme hooks, we would only be adding to the confusion by using a prefixed action (and it would have to be prefixed, since themes using it would not be accepted to the .org repo otherwise).
If/when core and the community can resolve #21506, we can certainly revisit the idea.
Oh, and support docs are now published here: http://jetpack.me/support/site-logo/