Make WordPress Core

Opened 9 years ago

Closed 9 years ago

Last modified 9 years ago

#35982 closed defect (bug) (invalid)

The new feature 'site logo' is not working on-the-fly (postMessage)

Reported by: sidati's profile sidati Owned by: westonruter's profile westonruter
Milestone: Priority: normal
Severity: normal Version: 4.6
Component: Customize Keywords:
Focuses: javascript Cc:

Description

Hi,

Ive been using the v 4.5-beta1-36748 the last days, and it seems great but there is an issue with the new feature "Site Logo".
After changing the transport to postMessage (code bellow), it keeps refreshing the page after adding/changing/removing the logo image :/

<?php
$wp_customize->get_setting( 'site_logo' )->transport = 'postMessage';

Best,
Sidati

Change History (11)

#1 @westonruter
9 years ago

  • Keywords reporter-feedback added
  • Milestone changed from Awaiting Review to 4.5
  • Owner set to westonruter
  • Status changed from new to accepted

This sounds an issue related to Selective Refresh (#27355), where the site logo gets rendered with PHP filters applied after an initial purely JS preview. I haven't been able to reproduce the issue with the entire preview not refreshing in Twenty Sixteen. Note that in Core, the custom_logo setting is already added with postMessage:

<?php
$this->add_setting( 'custom_logo', array(
        'theme_supports' => array( 'custom-logo' ),
        'transport'      => 'postMessage',
) );

Is your site logo including the custom-logo-link CSS class name as is output by get_custom_logo()? Selective refresh will be looking for an element with this class. Here is how the custom_logo partial is registered in WP_Customize_Manager::register_controls():

<?php
$this->selective_refresh->add_partial( 'custom_logo', array(
        'settings'            => array( 'custom_logo' ),
        'selector'            => '.custom-logo-link',
        'render_callback'     => array( $this, '_render_custom_logo_partial' ),
        'container_inclusive' => true,
) );

If Selective Refresh doesn't find the element indicated by the selector, it will trigger a refresh by default.

Two options I see for you to try:

1) Adjust the selector to match what your theme is actually using if you have modified the output of get_custom_logo:

<?php
add_action( 'customize_register', function( $wp_customize ) {
    $new_selector = '.my-custom-logo';
    $wp_customize->selective_refresh->get_partial( 'custom_logo' )->selector = $new_selector;
}, 20 );

2) Or, you can disable fallback_refresh entirely (which might be what we want to do in Core):

<?php
add_action( 'customize_register', function( $wp_customize ) {
    $wp_customize->selective_refresh->get_partial( 'custom_logo' )->fallback_refresh = false;
}, 20 );

The downside if the selector isn't fixed is that the “Shift+click” to edit won't work, and there will be an extra Ajax request to render the partial but there won't be anything to render it into, so it will be wasted.

Last edited 9 years ago by westonruter (previous) (diff)

#2 @sidati
9 years ago

  • Keywords reporter-feedback removed
  • Resolution set to invalid
  • Status changed from accepted to closed

Exactly it was a bad selector, also i didn't know the exiting of theses functins :

  • the_site_logo()
  • get_the_site_logo()
  • has_site_logo()

Thank you very mush

#3 @westonruter
9 years ago

  • Keywords reporter-feedback added
  • Resolution invalid deleted
  • Status changed from closed to reopened

@sidati can you provide more details though on how you specifically were able to create a custom logo that lacked the selector? If you didn't know about those functions, then how did you embed the logo in a way that it lacked the selector?

#4 @sidati
9 years ago

Hi @westonruter,

Sorry about the delay, it takes time to reproduce the bug cause i removed the old code and use the logo functions, anyway this is the function :

	function wp123_get_logo(){

		if (!$img_id = get_theme_mod('site_logo')) {
			return;
		}

		list($img_src) = wp_get_attachment_image_src($img_id, array(150, 50));

		?><a href="#">
			<img class="site-logo" src="<?php echo esc_url($img_src) ?>" alt=">">
		</a><?php
	}

then i call it in my header, remember this is for the 4.5 BETA 1, i didn't test it with the BETA 2, i tough it was the transport property so i add this line as i said in my ticket :

<?php
$wp_customize->get_setting( 'site_logo' )->transport = 'postMessage';

This will change the logo (image) on-the-fly but also the page will refreshed after that.


P.S. as you see the function return nothing if the site-logo not is set yet, so pick a logo then change it and you will see the issue if its an issue.

Last edited 9 years ago by sidati (previous) (diff)

#5 follow-up: @westonruter
9 years ago

  • Keywords reporter-feedback removed

@sidati thank you. OK, this confirms what I suspected. Again, the setting is added with postMessage by default, so you're setting it to postMessage made no difference. The problem was that you manually embedded the site logo using a class name other than the one output by the_site_logo(), and since no element in the document had the site-logo-link class, this then resulted in the partial's fallback logic of refreshing the entire preview to happen.

@obenland This relates to what we talked about in regards to having site title and tagline have default selective refresh irrespective of the theme. This was not implemented for site title and tagline, instead opting for each theme to explicitly add support, see #33738. I wonder if the same should be done for site logo, to remove the partial from being added in WP_Customize_Manager to instead add it explicitly by the theme?

#6 in reply to: ↑ 5 @obenland
9 years ago

Replying to westonruter:

I wonder if the same should be done for site logo, to remove the partial from being added in WP_Customize_Manager to instead add it explicitly by the theme?

I'm not sure I understand why. Title and tagline don't have a fix html/css-class structure while custom logo does. If themes choose not to use the custom logo api they should also adjust the selective refresh selector, but with the standardized API in place is themes using it when declaring support for it not a reasonable enough assumption to make?

#7 @westonruter
9 years ago

  • Milestone 4.5 deleted
  • Resolution set to invalid
  • Status changed from reopened to closed

@obenland good point :)

#8 @sidati
9 years ago

@westonruter
Just make all developers knows that there is a functions related to this feature the_site_logo(), get_the_site_logo() and has_site_logo(), so they make sure to avoid using there own markup :)

#9 follow-up: @dgwyer
9 years ago

I just tried the code:

add_action( 'customize_register', function( $wp_customize ) {
	$wp_customize->selective_refresh->get_partial( 'site_logo' )->fallback_refresh = false;
}, 20 );

And keep getting the error: "WARNING: CREATING DEFAULT OBJECT FROM EMPTY VALUE IN [path ommitted] ON LINE 15".

This is a local development setup of WordPress running WP 4.6-RC1-38170, and PHP 5.5.24. I assume the code is correct (it's exactly what Weston posted above). If so why am I seeing this error? I know I can ignore it but I'd prefer not to see any errors.

#10 in reply to: ↑ 9 @westonruter
9 years ago

Replying to dgwyer:

And keep getting the error: "WARNING: CREATING DEFAULT OBJECT FROM EMPTY VALUE IN [path ommitted] ON LINE 15".

@dgwyer the issue is that my comment was written when the feature was called “Site Logo”. Later in the release it was renamed to “Custom Logo”. I just updated my comment to reflect this change, and here is a more robust example you could use:

<?php
add_action( 'customize_register', function( $wp_customize ) {

        // Bail if not WP>=4.5.
        if ( ! isset( $wp_customize->selective_refresh ) ) {
                return;
        }

        // Bail if theme doesn't have Custom Logo partial (likely doesn't support it).
        $partial = $wp_customize->selective_refresh->get_partial( 'custom_logo' );
        if ( ! $partial ) {
                return;
        }

        $partial->fallback_refresh = false;
}, 20 );

#11 @dgwyer
9 years ago

That was it! Thanks Weston.

Note: See TracTickets for help on using tickets.