Pass post format as a class to TinyMCE init

[helen]

Previously: #19437 for post type.

[azaozz]

Does this need anything more than 23198.patch?

[helen]

Well, needs to be format, not status :)

[azaozz]

Argh, was thinking it would be good to have the post_status as a class there too... Then forgot to add the main bit, post format :)

[adamsilverstein]

my patch adds a little more error checking and sanitizing, and a post-standard-format fallback; i got the code basis from post-template.php; while there i also added some missing whitespace, sorry if that doesn't belong in this patch.

[azaozz]

Replying to adamsilverstein:

my patch adds a little more error checking and sanitizing, and a post-standard-format fallback;

As far as I see get_post_format() ​does not return WP_Error (phpdoc is wrong there?) and already checks whether the post type supports formats. Perhaps we should update get_body_class() in post-template.php.

[adamsilverstein]

i'm note sure if there is an advantage in returning WP_Error vs. returning false, maybe providing more information;

set_post_format ​does return WP_Error, maybe we should add these lines into get_post_format?

if ( empty($post) )

return new WP_Error('invalid_post', ('Invalid post'));

[adamsilverstein]

adds WP_Error return to get_post_format

[adamsilverstein]

add-WP_Error.diff adds the WP_Error return into get_post_format; I don;t really know if we need that check, or if we don't why we would want it in set_post_format; and the phpdoc needs updating.

[azaozz]

Replying to adamsilverstein:

Seems get_post_format() is called after we have the post, i.e. without passing an argument or passing $post->ID. However passing a non-existing post ID will throw a PHP notice. Not sure whether we need WP_Error there or simply:

if ( empty($post) )
    return $post;

This would need a now ticket in any case.

[adamsilverstein]

cleared up phpdoc

[adamsilverstein]

updated phpdoc - $post is optional, since get_post_format calls get_post immediately, get_post_format will work with a post id, a post object or no post at all (uses the current post)

[adamsilverstein]

reviewing the code, i don;t see how the WP_Error is helpful, but maybe its a forward thinking bit of code. curious to see what others think.

[azaozz]

Initially it was returning WP_Error [15777], which was changed in [16319] together with changing wp_get_object_terms() to get_the_terms().

[adamsilverstein]

so we can remove the check for WP_Error here, and then also remove that check from ​post-template.php?

[WraithKenny]

What about when the post format is changed while editing the post? Will the tinymce class be updated?

Also, are there classes that get added to body_class or post_class that should be added? Should we rather populate the init with post_class()?

[adamsilverstein]

we should probably use get_post_class, not sure why that wasn't used originally, that would ensure all the same classes were passed to tinymce as get passed in other spots like actual post display on site. i'll redo it that way. as for updating when you change the post format, yes, it will change, once you you click update, not before then - would you expect/want it to, and if so, why?

[azaozz]

Replying to adamsilverstein:

Opened #23255 for get_post_format(). Could you add your patches there.

[azaozz]

What about when the post format is changed while editing the post? Will the tinymce class be updated?

Good question. In the spirit of WYSIWYG-ness thinking we should change the body class in the Visual editor dynamically when the post format is changed.

Also, are there classes that get added to body_class or post_class that should be added? Should we rather populate the init with post_class()?

Not sure it's a good idea to add everything from post_class() to the body in TinyMCE. It is intended for the front-end and not intended for the body tag, so risking to bring compatibility problems. Also themes may not use post_class(). More advanced themes can add a body class to the editor with the init options for TinyMCE.

[helen]

This is related to post formats UI, so I imagine that dynamically adding/changing would be ideal in order to have a chance at doing a truer WYSIWYG preview. Adding the rest of post_class() is likely unnecessary and introduces even more problems with dynamic changes (sticky, category, etc.). Also not in the scope of this ticket :)

[adamsilverstein]

Replying to azaozz:

Replying to adamsilverstein:

Opened #23255 for get_post_format(). Could you add your patches there.

ok, i can do that.

[adamsilverstein]

Replying to helen:

This is related to post formats UI, so I imagine that dynamically adding/changing would be ideal in order to have a chance at doing a truer WYSIWYG preview. Adding the rest of post_class() is likely unnecessary and introduces even more problems with dynamic changes (sticky, category, etc.). Also not in the scope of this ticket :)

good points! i will try to cook up some jQuery to update the applied classes, fun!

[azaozz]

i will try to cook up some jQuery to update the applied classes, fun!

You will need to hook that up to TinyMCE. Something like:

if ( typeof tinymce != 'undefined ) {
  // get the new class
  jQuery('#post-formats-select input.post-format').on('change', function(){
    var ed = tinymce.get('content'), edBody, format;
  
    if ( ! ed )
      return;

    if ( this.id ) {
      edBody = ed.getBody();
      format = this.id; // also need to handle 'post-format-0'

      edBody.className = edBody.className.replace(/\bpost-format-[^ ]+/, '');
      ed.dom.addClass(edBody, format);
    }
  });
}

[adamsilverstein]

thanks, thats super helpful, seems like it... i'll try to insert that and give it a try. i was just going to traverse the dom down into the iframe but this makes much more sense.

question:

should post-format-0 even be there, that seems like a bug, shouldn't the 1st radio id be post-format-standard?

should i try to fix that or is there for legacy support of some kind, and if so should i catch that and rewrite in the js so the applied class is post-format-standard? i what will get_post_class() return?

(this - )
<input id="post-format-standard" class="post-format" type="radio" checked="checked" value="standard" name="post_format">
instead of
<input id="post-format-0" class="post-format" type="radio" checked="checked" value="0" name="post_format">

[azaozz]

The comment by @helen got me thinking: we may want to dynamically change/replace a TinyMCE body class for something else in the future. We can abstract this bit and add it to the 'wordpress' plugin for TinyMCE. That will make it available to all. I'll make a patch later today.

[adamsilverstein]

great idea. let me know if you want me to hook that into the format list on the edit page, and what about post-standard instead of post-0 there?

[helen]

Replying to azaozz:

The comment by @helen got me thinking: we may want to dynamically change/replace a TinyMCE body class for something else in the future. We can abstract this bit and add it to the 'wordpress' plugin for TinyMCE. That will make it available to all. I'll make a patch later today.

Anything on that?

[wonderboymusic]

OCD'd the whitespace for the new code

[adamsilverstein]

updated tinymce container class dynamically when changing post-format

[adamsilverstein]

23198.2.diff​ adds code to post-formats.js to swap out the post-format-(format) class added to the tinymce div; works with current tabs, easy to update when those go away. based on code suggestion from @azaozz

[azaozz]

Some quick notes on 23198.2.diff​:

• Still needs if ( typeof tinymce != 'undefined ) at the top.
• Not sure it needs post-type- body class as well... Nothing to do with formats :)
• Seems post-formats.js is loaded only on the Edit Post screen. Can be in post.js, no need to be separate file.

[adamsilverstein]

Replying to azaozz:

Some quick notes on 23198.2.diff​:

• Still needs if ( typeof tinymce != 'undefined ) at the top.
• Not sure it needs post-type- body class as well... Nothing to do with formats :)
• Seems post-formats.js is loaded only on the Edit Post screen. Can be in post.js, no need to be separate file.

thanks for the review, will update.

[helen]

Another question about the patch - why add the ID attribute for the slug? There's already a data-wp-format attribute.

[adamsilverstein]

Replying to helen:

Another question about the patch - why add the ID attribute for the slug? There's already a data-wp-format attribute.

will remove.

[adamsilverstein]

cleanup as per suggestions

[azaozz]

In 23730:

Pass post format as a class to TinyMCE's body, props adamsilverstein, fixes #23198