Twenty Fourteen: prefix the Featured_Content class and options

[kovshenin]

We talk a lot about prefixing all the things, so let's prefix the Featured_Content class in Twenty Fourteen, since it's not (yet?) a core thing, as well as the featured-content option.

Required: Themes must provide a unique slug for anything in the public namespace, including translation textdomain, all custom function names, classes, hooks, public/global variables, database entries (Theme options, post custom metadata, etc.) ​ref

I don't think Twenty Fourteen should be an exception to this rule.

[kovshenin]

First pass in 26233.diff​.

The featured_content_layout setting in the customizer should also probably be prefixed, regardless of the fact that it's a theme mod and not an option, because if a plugin declares a setting with the same name (could be an option, not a theme mod) there will still be a conflict because of how the customizer uses ids.

Also, since the option names have been changed (and are no longer compatible with other plugins) we can decide whether we want twentyfourteen-featured-content or twentyfourteen_featured_content, because right now we use - for options and _ for the theme mod and other places. I assume the - was for plugins compat.

[kovshenin]

See also: ​http://make.wordpress.org/core/2013/11/12/featured-content-epilogue/

[celloexpressions]

In the process, can we also remove the add_theme_support part? It seems really silly to add_theme_support( 'themeprefix-...' ). We could just set our values as the defaults and take the file inclusion as indication that the theme supports its own component.

[obenland]

The class should not be in Twenty Fourteen in the first place - the theme should only have to care about the display of featured content, not how it's managed. Bringing it in was a compromise to enable support for a slider that is smarter than using sticky posts. Using post tags is far from ideal, but it's the next best thing.

This is not an oversight. The reason to leave it unprefixed is to make it pluggable for existing/future featured content solutions, like the afore mentioned core plugin. It will also make Twenty Fourteen forward compatible if/when a FC solution gets into core.

Let's keep it as is and make it as easy as possible for plugins to enhance the functionality.

[DrewAPicture]

Replying to obenland:

The class should not be in Twenty Fourteen in the first place - the theme should only have to care about the display of featured content, not how it's managed. Bringing it in was a compromise to enable support for a slider that is smarter than using sticky posts. Using post tags is far from ideal, but it's the next best thing.

This sounds like an excellent argument in favor of not bundling it with the theme. If it's a plugin, great, ship it as a plugin. If it's part of the theme, ship it as part of the theme, but prefix it as such.

In my opinion, this isn't the same thing as bundling a library or similar. This is plugin functionality in a default theme, which seems weird to me. It also sends kind of an odd message to send to the theme community, because, really, that's what default themes do. They're supposed to be the shining examples of how to do it the right way. Is this compromise doing it the right way?

[dd32]

This feels like a case where if twentyforteen really requires this functionality, it should be moved into core instead of the theme.

I know that the Featured Content plugin missed the boat for core, but that should also mean it missed the boat for twentyforteen. If we're going to bundle it, it should be included in core as a minimal feature intended on a future iteration, rather than being bundled with the theme (and a bunch more themes as they start to use it too).

[kovshenin]

Whether FC will or will not be in core, bundling a plugin into the default theme sounds wrong. It's the exact opposite of what we tell other theme developers to do. If FC is a plugin, it should be a plugin, and Twenty Fourteen can declare support for that plugin, with a fallback to sticky posts. Just like the numerous existing themes on WordPress.org.

Let's not try and bend the "prefix all the things" and "plugin territory" rules for Twenty Fourteen.

[lancewillett]

Replying to kovshenin:

Whether FC will or will not be in core, bundling a plugin into the default theme sounds wrong. It's the exact opposite of what we tell other theme developers to do.

We don't have a choice -- the theme depends on it to operate, and the plugin didn't make the 3.8 cut -- see ​http://make.wordpress.org/core/2013/11/12/featured-content-epilogue/ and #24857 -- we need to put the functionality in the theme. Or in core, as @dd32 mentioned.

[lancewillett]

Matt as 3.8 lead asked for this functionality, we don't feel the theme will be Twenty Fourteen without it.