Removes hentry class from the array of post classes

[davidakennedy]

• Currently, having the class on pages is not correct use of hentry.
• hentry requires more properties than pages typically have.
• Core is not likely to remove class because of backward compatibility.
• See: https://core.trac.wordpress.org/ticket/28482 and http://microformats.org/wiki/hentry

Fixes #844

[dshanske]

Shouldn't the case be if type is 'post' so it doesn't automatically end up on custom post types?

[obenland]

If the CPT is syndicated it should continue having it.

[obenland]

@davidakennedy Could we add a blank line between the short and long description of the function and make "See" a "@see"?

[davidakennedy]

@obenland Adjusted the description based on your feedback.

Also, edited the CSS since we have one spot where we use .hentry. I think using .post and .page will be sufficient, but it's good to think that through.

Shouldn't the case be if type is 'post' so it doesn't automatically end up on custom post types?

@dshanske I actually started going this route, but went with page instead because we don't know how authors want to use their post types. I think they need to be able to make the call of whether or not they're syndicated, and we shouldn't get in the way.

[sixhours]

This patch works for me. I vote we merge it.

[obenland]

I wonder if we should check for if the current post type has archive enabled, as a way to find out if it's a syndicated post type.

Just as a heads up, this change will break compatibility with whatever relies on that class being there all the time. And since this is core output we will forever carry that "incompatibility" layer, opposed to when we add a shiv to make up for something missing in core.

[dshanske]

As someone who spent a lot of time trying to figure out this issue, I don't know if there is any other way. I think something like a starter theme should be pushing things forward.

[benoitchantre]

@obenland Maybe something could be done with add_theme_supportor remove_theme_support to let Wordpress know that the theme doesn't use the hentryclass to style elements.

[dshanske]

I have this in ticket #30783

[dshanske]

It seems in the open core ticket, there was a suggestion that theme options were somehow undesirable

[dshanske]

How do we finally decide about merging this?

[phoenixenero]

Maybe add a setting in the Theme Customizer under "Theme Options>Advanced" or some similar panel?

"Re-enable .hentry class for pages."
"Enable this if one or more of your plugins fail to work on pages."

Or something similar.

Bonus points: we get to show how to add settings to the Theme Customizer.

[dshanske]

Core has closed the ticket to address this in Core due backcompat. I would consider that a declaration this is a theme issue, not a Core issue.

[davidakennedy]

I'd be in favor of merging this, especially since Core is not going to do anything with regards to Microformats 2. Plus, Core can't really fix this issue because of breaking current themes and plugins. So it's up to themes to correct the issue and educate.

I wonder if we should check for if the current post type has archive enabled, as a way to find out if it's a syndicated post type.

@obenland Do you mean if a custom post type has has_archive set to true?

[dshanske]

Think I should reopen a PR on adding Microformats 2 to _s?

[philiparthurmoore]

Pretty much fully in agreement with @davidakennedy about merging.

[grappler]

@davidakennedy Do you think you could update this PR`?

We could remove the class from all of post types that are not hierarchical.

if ( ! is_post_type_hierarchical( get_post_type() ) {
	$classes = array_diff( $classes, array( 'hentry' ) );
}

As this will be only added on new themes it is like we are breaking backwards compatibility. If there are any compatibility issues between new themes and plugins then that will show up a place for improvement in the plugin.

[davidakennedy]

@grappler I refreshed the PR, but kept the existing code. I'm confused because if ( ! is_post_type_hierarchical( get_post_type() ) wouldn't remove hentry from pages. Unless I'm missing something.

I do think it would be ideal if this could be applied to all post types that don't need this class.

Also, happy to squash commits once we figure out the best path.

[grappler]

@davidakennedy I have been thinking about this a bit more.

Sorry if ( is_post_type_hierarchical( get_post_type() ) {} would be the correct way. So if the post type is hierarchical then it would remove hentry.

But on the other hand it depends on what the theme outputs in the markup. If a theme has hentry for a post then it must include entry-title, updated and author. I think we need to discuss a bit more how we can handle this.

One idea would be to remove it for all post types except posts. Depending on what plugins the themes supports it can make some changes. It maybe an idea to add a standard filter that all plugins could use. We might want to look at see how it affects the Post Types in Jetpack. Another idea would be to use add_theme_support() to define support for it.

My suggestion is till we can answer these questions we should make the changes to the CSS and then create a separate PR to decide when the hatom class should be added.

[davidakennedy]

@grappler This definitely is tricky.

My suggestion is till we can answer these questions we should make the changes to the CSS and then create a separate PR to decide when the hatom class should be added.

I agree. I'll open a separate issue for this.

This certainly seems complex. 😄 I'm wondering if we only deal with the post types we know about, the default ones in WordPress. Then explain why we did what we did in the code comment, and provide recommendations depending on what a post type does. I'd rather do that than take guesses or have to add a lot of complexity to a starter theme. Is there a way we could advance best practices without having to code for every situation? Because that seems hard here thanks to all the variables.

[jdevalk]

Just a note, came here through a discussion on the WP Slack, and agree with what was said there: JSON-LD has the future, hentry is hard, so it should be removed from core. Have created a ticket for that in core:

https://core.trac.wordpress.org/ticket/41711

[davidakennedy]

Closing in favor of the path outlined in #1268.