switch thematic_body_class() and thematic_post_class() to body_class() and post_class()

[GoogleCodeExporter]

What steps will reproduce the problem?
1. add a custom taxonomy
2. add taxonomy terms to a post
3. visit the single post 

What is the expected output? What do you see instead?
The body classes should include s-tax-TAXONOMY and s-TAXONOMY-TERM, and the 
post classes should have p-tax-TAXONOMY and p-TAXONOMY-TERM too. They are 
missing.

What version of the product are you using? On what operating system?
Thematic 0.9.7.8 development version

Please provide any additional information below.

In dynamic-classes, a check is made before adding the taxonomy classes:
if ( isset($single_post_type->taxonomy) )

Doing a print_r on $single_post_type->taxonomy gives
PHP Notice:  Undefined property: stdClass::$taxonomy
even though there are taxonomy terms set.

Turns out the correct property is 'taxonomies', not 'taxonomy'.

Original issue reported on code.google.com by invistr...@gmail.com on 4 Nov 2011 at 11:21

Attachments:

• dynamic-classes.diff

[GoogleCodeExporter]

A bit of testing  reveals other issues with this code. The property 
$single_post_type->taxonomies seem to always be set with an empty array, 
regardless of whether there are taxonomy terms associated with the object or 
not. I wonder if this is expected/intended behavior of wordpress or not, but 
the fact remains. The issues I see are two:

1. For some reason, is_single() evaluates to true on attachment pages. By 
default, attachments do not have any taxonomies associated with them. This 
results in php error notices of "Trying to get property of non-object" when 
visiting an attachment page.

2. The code is only removing tags and categories from the results. This means 
that a child theme that activates post formats will get post classes of 
p-tax-post_format and p-post_format-post-format-FORMAT. This may be intentional 
but i think the post format css classes should be simpler. 

I suggest this instead: check if there are any terms associated with the post, 
and only add taxonomy classes for taxonomies that are not builtin. I will start 
a new issue regarding dynamic classes for post formats and custom post types.

Original comment by invistr...@gmail.com on 5 Nov 2011 at 11:13

Attachments:

• dynamic-classes-1.diff

[GoogleCodeExporter]

Original comment by eugene.m...@gmail.com on 17 Jan 2012 at 8:31

• Changed state: Accepted

[GoogleCodeExporter]

somewhat related to 
http://code.google.com/p/thematic/issues/detail?id=122&q=dynamic

is there a reason thematic_body_class doesn't just filter into wp's body_class 
in lieu of trying to recreate everything itself?  body_class() already does 
post types, taxonomy terms and post formats.  

from my local test site a sampling of thematic's body classes on a single post 
of type 'prism_portfolio'  

wordpress  
blogid-1  
y2012  
m01  
d18
h02
singular
slug-test-portfolio-item
single
postid-696
s-y2012
s-m01
s-d18
s-h02
s-author-helga
s-comments-open
s-pings-open
loggedin
windows
chrome
ch16

frankly i have never used MOST of those... certainly not any of the date-based. 
 why is wordpress a class?  the default WP classes are actually more useful to 
me since it has the post type in it.  

the default wp body classes on the same post:

single
single-prism_portfolio
postid-696
logged-in
admin-bar
windows
chrome
ch16

almost all the WP classes are in the thematic classes, though single is 
singular and logged-in is loggedin.  

Original comment by helgathe...@gmail.com on 18 Jan 2012 at 3:13

[GoogleCodeExporter]

To filter wp's body class sound good to me. We get classes for post-formats, 
CTP, CTP archive pages and custom taxonomies.

A quick note though. Singular ≠ single. That class is for the template tag 
is_singular(), which is true on any single posts, attachments and pages. As you 
can see, single is also in the thematic classes and singular is one of the 
thematic extras.

And I think the last three - windows, chrome and ch16 - is not in wordpress but 
from thematic, already added via filtering WP's body_class . Seems a bit 
counter-intuitive to have _some_ classes added via filtering and others not. 

I think the simplest way would be to use

body_class( thematic_body_class() )

in the templates and have the function returning empty when 
THEMATIC_COMPATIBLE_BODY_CLASS is false. But filter works as well.

Original comment by invistr...@gmail.com on 18 Jan 2012 at 7:01

[GoogleCodeExporter]

I agree. Here are my thoughts & concerns:

Creating a filter presents problems. String replacement will always have a 
dependency on the WP function output.

We couldn't ever remove WP attributes by default without informing the user of 
the choice to do so. 

What happens when we have added a class that later WP decides to add a a 
similarly intended one. 

I'm thinking backward compatibility without foreword maintenance and 
observation of the core WP function is unlikely. 

Original comment by eugene.m...@gmail.com on 18 Jan 2012 at 8:17

[GoogleCodeExporter]

Original comment by eugene.m...@gmail.com on 22 Jan 2012 at 9:23

• Changed title: Dynamic Classes: body and post classes for custom taxonomies are not being added

[GoogleCodeExporter]

Anyone moving on this currently? I'm thinking I'll take a stab at the filter. 

Original comment by eugene.m...@gmail.com on 22 Jan 2012 at 11:05

[GoogleCodeExporter]

Original comment by eugene.m...@gmail.com on 23 Jan 2012 at 1:05

• Changed title: body and post classes for custom taxonomies are not being added
• Added labels: DynamicClasses

[GoogleCodeExporter]

been waiting on a little feedback.  might take a stab tomorrow night.  

am thinking invistr's suggestion of body_class( thematic_body_class() ) is the 
way to go... with thematic_body_class() returning thematic's special classes if 
THEMATIC_COMPATIBLE_BODY_CLASS is true and nothing if 
THEMATIC_COMPATIBLE_BODY_CLASS is false (which would just get you WP's default 
body classes)

Creating a filter presents problems. String replacement will always have a 
dependency on the WP function output.
**am thinking we just filter WP's body_class,  not sure where string 
replacement comes into the picture

We couldn't ever remove WP attributes by default without informing the user of 
the choice to do so. 
**i think we should just fall back to WP's default, and only add-on if the user 
wishes 

What happens when we have added a class that later WP decides to add a a 
similarly intended one. 
**so worst case we end up with single and singular ?  if WP is adding stuff by 
default, then it might evolve that child-themers don't need to enable thematic 
classes and it could be phased out.

I'm thinking backward compatibility without foreword maintenance and 
observation of the core WP function is unlikely. 
**well we're already looking at a lot of forward maintenance just to get 
thematic_body_class to be up to date with taxonomies, post types, post type 
archives and post formats

Original comment by helgathe...@gmail.com on 23 Jan 2012 at 3:52

[GoogleCodeExporter]

Yep. We should filter the WP function to add in the Thematic's "legacy" 
classes. 

"Legacy" is the function namespace to use when deprecating and providing 
backward compatiblity for Thematic functions.

How bout this: We accept phasing out the bulk inclusion of the old classes and 
work towards a granular filtering of the WP default. 

Three modes to the filter:

1. default: not filtered... vanilla WP classes
2. all of the legacy thematic classes get added to WP's 
3.  a la cart. child themes could pick and choose which classes to add via 
current_theme_suppports()

Using add_theme_support is preferable to using boolean filter "switches" and 
constants imo.

Original comment by eugene.m...@gmail.com on 23 Jan 2012 at 5:11

[GoogleCodeExporter]

wouldn't 3. just be the child theme filtering its own values into the wp 
body_class filter?  i'm not sure i see the need for thematic to support 
granular filtering.  it just seems like an added level of complication

agree on add_theme_support w/ current_theme_supports().  didn't realize you 
could use that for your own custom purposes.  i like it.  

Original comment by helgathe...@gmail.com on 23 Jan 2012 at 7:34

[GoogleCodeExporter]

obviously a rough start, but my first crack at switching thematic_body_class() 
function to a filter

Original comment by helgathe...@gmail.com on 23 Jan 2012 at 8:55

Attachments:

• dynamic-classes.php.patch

[GoogleCodeExporter]

I also wonder about the use of granular filtering. Are people really using all 
the "thematic_show_bc_xxx" filters?

Good start. Now there is the detective work of removing the classes that 
thematic duplicates from WP and keeping only the add-ons. If we care that is - 
legacy is legacy. But duplicate classes seem a bit unnecessary to me.

AFAIK there are at least (only?) two classes that have the same functionality 
but different wording between WP and thematic.

WP: error404 vs Thematic: four04
and
WP: logged-in vs Thematic: loggedin

But for example I know that the attachment classes are exactly the same so 
those from thematic are redundant in that case. 

@Gene
 So is the idea to migrate to child themes using add_theme_support() in their functions.php rather than defining constants true or false? I like that.

I don't think it could be used for all of the current "switch" filters though. 
Some of them are on by default with a filter for themes to optionally turn them 
off. Don't know if you can do that with current_theme_supports. And I think it 
would be cleaner with using current_theme_supports for the major 
functionalities and retain some of the filter switches for the details, rather 
than child themes needing to add 64 add_theme_support's for every little thing. 

Original comment by invistr...@gmail.com on 23 Jan 2012 at 11:49

[GoogleCodeExporter]

Oh, and I'm middlesister. :-)
Got a different handle on gmail.

Original comment by invistr...@gmail.com on 23 Jan 2012 at 11:52

[GoogleCodeExporter]

heya middlesister!   google cuts off half the handle too so couldn't even get 
your whole handle

@"I also wonder about the use of granular filtering. Are people really using 
all the "thematic_show_bc_xxx" filters?"

i didn't even know they were there, so i am going to make the assumption that 
most general users aren't using them at all.  

a backwards compatibility that i didn't consider in my first patch is for child 
theme users who are currently filtering thematic_post_class.  maybe that 
can/should still be included?  

single and singular are about the only other similar ones.  i removed redundant 
classes from output via array_unique(), but i suppose they could be removed 
from the code entirely with a little backwards detective work.  

working w/ my patch, i currently have my child functions.php using

add_theme_support('thematic_legacy_body_classes');

i agree with you in opposing needing to add_theme_support multiple times for 
each granular level of body class... b/c i suspect we'll be porting this same 
thing to post classes and comments classes next and the number of 
add_theme_support calls will quickly get out of hand.  

Original comment by helgathe...@gmail.com on 24 Jan 2012 at 12:00

[GoogleCodeExporter]

Ah, sorry I missed that. Well then I think time is better spent on other 
patches than figuring out which code can be deleted and which should be kept. 
By declaring all of them legacy, it is clear that we are moving on anyway.

But I think you are right in that we should include the thematic_body_class 
filter for backwards compatibility. Maybe put the filter just before the 
array_unique.

add_theme_support('thematic_legacy_body_classes') works well for me too.

The only funny thing is that 'single' appears twice because thematic is not 
adding it as a separate item in the array. It puts the 'single' and the 
'postid-#' class in the same slot. Change it if you like, but it is minor.

Original comment by invistr...@gmail.com on 24 Jan 2012 at 10:41

[GoogleCodeExporter]

yeah, i'm ok w/ leaving the somewhat redundant ones there since they are legacy 
and not added by default.  

filter added.  split single into its own array entry so it should get caught by 
array_unique

Original comment by helgathe...@gmail.com on 24 Jan 2012 at 10:52

Attachments:

• dynamic-classes.php.patch

[GoogleCodeExporter]

This is not a priority heading into 0.9.8 for theme review. We can take our 
time here.

Original comment by eugene.m...@gmail.com on 26 Jan 2012 at 2:09

[GoogleCodeExporter]

i suppose we can punt this to thematic 1.0, even though i was so proud of 
myself for this patch.  ;)  the 1.0 update should be easier to get through 
review once we get 0.9.8 through considering the 1 year of changes since the 
last update of the official repo.  

Original comment by helgathe...@gmail.com on 26 Jan 2012 at 4:27

[GoogleCodeExporter]

Just worked my way through this discussion and without grasping all the details 
can see there's a revamp coming up.

I'm in the process of updating a site to test drive the latest rev - the same 
site from whence came the original bug report - and was wondering if 
Middlesister's fix: 'taxonomy' to 'taxonomies' (on lines 125 & 458 in current 
rev of dynamic-classes.php) is worth implementing in the interim? 

Awesome work by the way you three - wish I could be of more help in thrashing 
out code etc - but v. instructional to tag along :)

Original comment by umbert...@gmail.com on 1 Mar 2012 at 1:03

[GoogleCodeExporter]

now that we're at 1.0, should we be considering the punt return?  do you think 
we need to maintain the thematic_body_class filter... just with a deprecation 
notice? and then delete it the next time?  

i figure we're going to break a whole bunch more child themes as people have to 
switch from those constants to add_theme_support()  

Original comment by helgathe...@gmail.com on 15 Jun 2012 at 7:56

[GoogleCodeExporter]

if we switch thematic_body_class to something that filters body_class() then do 
we need to have childtheme_override_body_class() ?  same for post_class?  leave 
for know and wrap with a deprecation notice?  b/c if thematic_body_class is 
being deprecated, then surely its override is too?

Original comment by helgathe...@gmail.com on 17 Jun 2012 at 3:25

[GoogleCodeExporter]

monster patch.  switching to body_class() and post_class().  
thematic_body_class and thematic_post_class are still available to filter, so 
that shouldn't break.  both are added if enabled via add_theme_support() and 
for backcompat add_theme_support for the legacy classes is added if the 
traditional constant is defined.  figure we can do away with that down the road 
after we get the word out a bit more.  only thing i am not sure of is if we 
should turn the legacy classes on by default?  but perhaps if we leave the 
check for the THEMATIC_COMPATIBLE_BODY_CLASS to trigger the add_theme_support 
that will avoid breaking any styles that were dependent on the 
thematic-specific body classes. 

patch does not include changes to sample child theme, nor added strings to the 
.pot file (b/c poedit is kicking my butt) 

Original comment by helgathe...@gmail.com on 17 Jun 2012 at 3:54

Attachments:

• body_class_and_post_class.patch

[GoogleCodeExporter]

few tweaks that separate some classes that were strings with spaces. making 
them each their own array should cause array_unique() to pick up the duplicates 

Original comment by helgathe...@gmail.com on 17 Jun 2012 at 4:11

• Changed title: _switch thematic_body_class() and thematic_post_class() to body_class() and post_class() _

Attachments:

• dynamic-classes.php.patch

[GoogleCodeExporter]

Thematic development has moved to github. Please report future issues here: 
https://github.com/ThematicTheme/Thematic/

This change was made in the 1.0.3 release.

Thanks!

Original comment by eugene.m...@gmail.com on 28 Oct 2012 at 10:01

• Changed state: Fixed