Add compatibility with themes and plugins #123
Conversation
|
@valendesigns I think we should have a separate class for each theme/plugin adding compatibility for. Eventually the theme/plugin should copy the compatibility into their own codebases, and then we can just delete our classes. |
|
@westonruter We could do it that way, sure. One thing that is not exactly clear though with partials is how to extend the JS and replace the selector for themes. Should we use an event or is there a way to override the selector in a simpler way? |
|
@valendesigns That's a good question. I think that we can have a registry of the partials defined in PHP, with the selectors and render callbacks for each. And then the theme can filter this array. Or rather, it can be perhaps a theme support feature. That being the case, we can actually register the partials with PHP statically instead of dynamically with JS, or there can be a hybrid where theme partial configuration is fed into the filter for dynamic partials but the selectors and configurations (e.g. |
|
|
||
| foreach ( array( 'theme', 'plugin' ) as $type ) { | ||
| foreach ( glob( dirname( __FILE__ ) . '/' . $type . '-support/class-*.php' ) as $file_path ) { | ||
| if ( is_readable( $file_path ) ) { |
There was a problem hiding this comment.
I think the is_readable() is unnecessary?
| @@ -39,23 +40,26 @@ | |||
| // Post field partial for post_title. | |||
| partial = new api.previewPosts.PostFieldPartial( id + '[post_title]', { | |||
| params: { | |||
| settings: [ id ] | |||
| settings: [ id ], | |||
| selector: api.previewPosts.data.partialSelectors.title || '.entry-title' | |||
There was a problem hiding this comment.
Good that you added a fallback selector. But I think it has to include the baseSelector here as well.
There was a problem hiding this comment.
I can move it from customize-post-field-partial.js to here.
westonruter
changed the title
| */ | ||
| public function show_in_customizer() { | ||
| if ( Jetpack::is_module_active( 'contact-form' ) ) { | ||
| get_post_type_object( 'feedback' )->show_in_customizer = false; |
There was a problem hiding this comment.
For good measure, I'd probably make sure that the feedback is actually registered. For example:
if ( ! Jetpack::is_module_active( 'contact-form' ) ) {
return;
}
$post_type_object = get_post_type_object( 'feedback' );
if ( ! $post_type_object ) {
return;
}
$post_type_object->show_in_customizer = false;
|
@valendesigns one more thought: instead of having a default list of the post field partials that a theme is expected to support and the selectors for each, should these configurations not actually be added to |
|
@weston That is a thought I had, but wanted to get some more input from you before going much further. with this PR. Essentially we need to come up with the best way to pass the partial config to the JS so it can be be added dynamically. You touched on that in one of the comments above, though that means we are relying on the support class to tell us which partials are supported. Which makes complete sense, we just need a good reusable schema to make this work well. |
|
@westonruter So we need to let the support class dictate the config, which is exported to JS and used to set the |
|
Also if the theme has not set a config then does that mean we should not register any partials? |
There can be a method like
If the theme doesn't have a config set for partials, then we can just register the defaults which are supported by 99% of themes. For example any partials that target |
|
@westonruter I added the defaults which themes can unregister via the filter or modify as they see fit. I feel the defaults are fairly standard across many of the Twenty Something themes, there will be minor tweaks I'm sure, but overall themes should be using these partials. Have you seen the latest commit? |
| 'container_inclusive' => true, | ||
| ), | ||
| 'post_author' => array( | ||
| 'biography' => array( |
There was a problem hiding this comment.
Do you think biography is standard enough to be a default?
There was a problem hiding this comment.
I suppose bio is not going to be in all themes, we can remove it from the default list.
| add_customize_posts_support( 'Customize_Posts_Twenty_Ten_Support' ); | ||
| } | ||
| add_action( 'after_setup_theme', 'twentyten_support' ); | ||
|
|
There was a problem hiding this comment.
I think this might be a better approach:
add_action( 'customize_register', function ( $wp_customize ) {
if ( isset( $wp_customize->posts ) ) {
$wp_customize->posts->add_support( 'Customize_Posts_Twenty_Ten_Support' );
}
});or
add_action( 'customize_register', function ( $wp_customize ) {
if ( isset( $wp_customize->posts ) ) {
$wp_customize->posts->add_support( new Customize_Posts_Twenty_Ten_Support( $wp_customize->posts ) );
}
});
Fixes #82
Fixes #103