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

Site Logo Block: logo data source and block UX #32065

Closed
creativecoder opened this issue May 20, 2021 · 7 comments · Fixed by #32229
Closed

Site Logo Block: logo data source and block UX #32065

creativecoder opened this issue May 20, 2021 · 7 comments · Fixed by #32229
Labels
[Block] Site Logo Affects the Site Logo Block [Type] Discussion For issues that are high-level and not yet ready to implement.

Comments

@creativecoder
Copy link
Contributor

The Site Logo block is planned for release in WordPress 5.8.

It was created to use a dedicated option (site_logo) that interoperated with the custom-logo theme_mod used in the Customizer:

  • If a site_logo option is set from the block, that logo is shown in the Customizer in place of the theme_mod (by hooking into theme_mod_custom_logo)
  • If the custom-logo theme mod is updated or deleted in the Customizer, that logo is copied into the site_logo option or the site_logo option is deleted

Related code


In preparation for merging Gutenberg into Core for the 5.8 release, the block was revised to use the custom-logo theme_mod directly, rather than a separate option. (I believe this was prompted by a failing Core unit test, but I don't know the full context.)

This has a negative impact on the UX of the Site Logo block, in that the logo now disappears from the block when switching themes (because the block's logo setting is now theme specific).

logo-disappears-when-switching-themes.mov

While this is considered normal for the Custom Logo option in the Customizer, I'm not convinced it's an acceptable experience for the Site Logo block. Having the logo setting disappear when switching themes seems broken to me. I'm hoping we can improve the UX before the block is released in Core.

I'm creating this issue to bring awareness of the change, it's impact, and discuss what the best path forward for the Site Logo block in WP 5.8.

How should the site logo block setting be stored?

What's the best way to balance backward compatibility with the custom-logo theme_mod and create good experience of using the block (including when switching themes)?

What can we achieve before the 5.8 release?

@creativecoder creativecoder added [Type] Discussion For issues that are high-level and not yet ready to implement. [Block] Site Logo Affects the Site Logo Block labels May 20, 2021
@aristath
Copy link
Member

I feel it should be mentioned here that changing themes is not something that a site-owner will do often.
Themes in WordPress are set-and-forget, and when a site-owner switches themes it's because they are either starting a new site (in which case they switch themes to find one that suits them), or they are redesigning/refactoring their existing site. In both cases, adding a logo is trivial after the user has made up their mind about which theme they want to use.

That being said, if we feel that we want the logo to persist when switching themes, then we don't need to change the way logos get saved in WP... We can simply add a check, and if the old theme has a logo but the new one does not, then copy-over the logo to the new theme. It's a lot less intrusive as an implementation...
Adding this code does the trick (tested and confirmed)

add_action( 'switch_theme', function( $new_name, $new_theme, $old_theme ) {
    $old_theme_slug = $old_theme->get_stylesheet();
    $new_theme_slug = $new_theme->get_stylesheet();

    $old_theme_mods = (array) get_option( "theme_mods_$old_theme_slug" );
    $new_theme_mods = (array) get_option( "theme_mods_$new_theme_slug" );

    if ( isset( $old_theme_mods['custom_logo'] ) && $old_theme_mods['custom_logo'] && ( ! isset( $new_theme_mods['custom_logo'] ) || ! $new_theme_mods['custom_logo'] ) ) {
        $new_theme_mods['custom_logo'] = $old_theme_mods['custom_logo'];
        update_option( "theme_mods_$new_theme_slug", $new_theme_mods );
    }
}, 10, 3 );

@youknowriad
Copy link
Contributor

I added to the 5.8 board to at least get a conclusion/decision here before 5.8. Who knows the historic here about custom logo and why it was a theme specific setting in the first place?

Pinging some folks to have more thoughts. cc @jasmussen @azaozz @mtias @westonruter

@westonruter
Copy link
Member

I recall that it's theme-specific mainly because it may depend on a theme's layout. If you have an area in the theme for the logo that is 400x200 pixels, but then switch to another theme that is designed to accommodate a 600x200 image size, then the image you picked for one theme won't just work in the other theme. So the intention was for the user to re-select the logo to re-crop it as required.

The Custom Logo feature was ported from Jetpack's Site Logo implementation.

I just checked Make/Core and confirmed the rationale:

In core, it’d be a theme mod enabled via add_theme_support( 'site-logo', size ), rather than storing the logo persistently across themes. This is for a few reasons:

  • Customizer controls should only be visible when a feature is supported by the theme.
  • Prevent plugins from using a “global” logo when the Customizer controls may not be visible
  • Prevent poor display of logos that looked good on one theme, but whose size is not appropriate for another theme’s declared size.

@mtias
Copy link
Member

mtias commented May 21, 2021

I'd really love for us to get a bit further away from the theme-specific situation since we have an opportunity to start making the overall experience better and more meaningful, not tied to the constrains we had before. A theme can specify a desired size in the block attributes for their logo block, and we could make decisions based on whether the current logo the user has fits or not. Removing it entirely is a poor user experience.

@creativecoder
Copy link
Contributor Author

when a site-owner switches themes it's because they are either starting a new site (in which case they switch themes to find one that suits them), or they are redesigning/refactoring their existing site.

The fact that users who are new(er) to WordPress are more likely to switch themes and thus more likely to encounter the logo disappearing from the block makes it more of a problem in my mind: I suspect that they will experience it as instability or unreliability in a platform for which they haven't yet built up trust or understanding.


Can anyone provide more context on what conflicts the site_logo option and associated hooks were causing with Core? I'm happy to look at other ways to fix that problem.

@westonruter
Copy link
Member

Can anyone provide more context on what conflicts the site_logo option and associated hooks were causing with Core? I'm happy to look at other ways to fix that problem.

There is no site_logo option. Rather, there is a custom_logo theme mod. The possible conflicts are due to differences the desired image dimensions for logos across themes.

@creativecoder
Copy link
Contributor Author

@westonruter Sorry to be unclear--I mean the site_logo option that was used in the Site Logo block in the Gutenberg plugin, until it was changed in #31994 . I'm looking for more context on the technical reasons for that change.

@aristath aristath linked a pull request May 26, 2021 that will close this issue
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Site Logo Affects the Site Logo Block [Type] Discussion For issues that are high-level and not yet ready to implement.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants