Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add customizer theme support #952

Merged
merged 7 commits into from
Feb 22, 2018
Merged

Conversation

westonruter
Copy link
Member

@westonruter westonruter commented Feb 10, 2018

  • Make sure sanitizer is run in rendered partials.
  • Add AMP component scripts when selective refresh results in a new component added. Irrelevant since AMP apparently does this automatically.
  • Make sure scripts are allowed to be output when is_customize_preview(). Scripts need to be enqueued regardless of the user being in the iframe since frontend previewing depends on JS injecting the customize_changeset_uuid into requests.
  • Suspend CSS sanitizer to facilitate JS-based live previews.
  • Suspend limit on CSS size so that customize-preview.css can be included.

Fixes #901.

@westonruter westonruter added this to the v0.7 milestone Feb 10, 2018
@westonruter
Copy link
Member Author

@DavidCramer FYI: Note the re-organization here for form handling. This will probably impact #936

@DavidCramer
Copy link
Contributor

Thanks @westonruter .

@westonruter westonruter changed the title [WIP] Add customizer theme support Add customizer theme support Feb 12, 2018
Copy link
Collaborator

@ThierryA ThierryA left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good step forward towards separating Theme Support from the legacy templates. I left a few comments which may be addressed depending on the answers, otherwise this is good to go once merge conflicts will be reloved.

@@ -178,6 +207,7 @@ public static function register_hooks() {
*/
add_action( 'template_redirect', array( __CLASS__, 'start_output_buffering' ), 0 );

// Commenting hooks.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

During the upcoming cleanup, I would suggest to move comments, widgets etc. in their on classes.

add_filter( 'comment_post_redirect', function() {
// We don't need any data, so just send a success.
wp_send_json_success();
}, PHP_INT_MAX, 2 );
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why passing 2 args here, I believe this is perhaps a left over?

}, PHP_INT_MAX, 2 );
self::handle_xhr_headers_output();
} elseif ( ! empty( self::$purged_amp_query_vars['_wp_amp_action_xhr_converted'] ) ) {
add_filter( 'wp_redirect', array( __CLASS__, 'intercept_post_request_redirect' ), PHP_INT_MAX, 2 );
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why passing 2 args here, I believe this is perhaps a left over?


// Dequeue styles unnecessary unless in customizer preview iframe when editing (such as for edit shortcuts).
if ( ! self::is_customize_preview_iframe() ) {
wp_dequeue_style( 'customize-preview' );
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about just doing wp_deregister_style( 'customize-preview' ); so that we don't have to loop through dependencies?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That could work, but it would also result in something like Query Monitor issuing a warning about a dependency missing.

@westonruter westonruter merged commit ac753c4 into develop Feb 22, 2018
@westonruter westonruter deleted the add/customizer-theme-support branch February 22, 2018 17:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants