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: 5.8 Compat #20047

Closed
wants to merge 7 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -73,22 +73,20 @@ function callback( $path = '', $site_id = 0 ) {
}

if ( isset( $args['id'] ) ) {
$logo_settings['id'] = (int) $args['id'];
}
if ( isset( $args['url'] ) ) {
$logo_settings['url'] = $args['url'];
}
if ( isset( $args['url'] ) || isset( $args['id'] ) ) {
update_option( 'site_logo', $logo_settings );
update_option( 'site_logo', (int) $args['id'] );
}

return $this->get_current_settings();
}

function get_current_settings() {
$logo_settings = get_option( 'site_logo' );
if ( ! is_array( $logo_settings ) ) {
$logo_settings = array();
$logo_settings = array();
$option = get_option( 'site_logo' );
if ( is_numeric( $option ) ) {
$logo_settings = array(
'id' => (int) $option,
'url' => wp_get_attachment_url( $option ),
);
}
return $logo_settings;
}
Expand Down
15 changes: 10 additions & 5 deletions projects/plugins/jetpack/modules/theme-tools/site-logo.php
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,18 @@
* Activate the Site Logo plugin.
*
* @uses current_theme_supports()
* @since 3.2
* @since 3.2.0
* @since 9.9.0 Uses Core site_logo option format universally.
*/
function site_logo_init() {
// For transferring existing site logo from Jetpack -> Core
if ( current_theme_supports( 'custom-logo' ) && ! get_theme_mod( 'custom_logo' ) && $jp_logo = get_option( 'site_logo' ) ) {
set_theme_mod( 'custom_logo', $jp_logo['id'] );
delete_option( 'site_logo' );
// For transferring existing site logo from Jetpack -> Core.
if ( current_theme_supports( 'custom-logo' ) && ! get_theme_mod( 'custom_logo' ) && get_option( 'site_logo' ) ) {
$jp_logo = get_option( 'site_logo' );
Comment on lines +30 to +31
Copy link
Member

Choose a reason for hiding this comment

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

Would it make sense to move the get_option here?

Suggested change
if ( current_theme_supports( 'custom-logo' ) && ! get_theme_mod( 'custom_logo' ) && get_option( 'site_logo' ) ) {
$jp_logo = get_option( 'site_logo' );
$jp_logo = get_option( 'site_logo' );
if ( current_theme_supports( 'custom-logo' ) && ! get_theme_mod( 'custom_logo' ) && $jp_logo ) {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I thought about that, but didn't because it would mean a db call that might not be needed if current_theme_support was false.

// Is the Site Logo in the old Jetpack format?
if ( ! empty( $jp_logo['id'] ) ) {
set_theme_mod( 'custom_logo', $jp_logo['id'] );
update_option( 'site_logo', $jp_logo['id'] ); // This is the convention used by the Site Logo block, part of WP 5.8+.
}
}

// Only load our code if our theme declares support, and the standalone plugin is not activated.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ public function enqueue() {
public function has_site_logo() {
$logo = get_option( 'site_logo' );

if ( empty( $logo['url'] ) ) {
if ( empty( $logo ) ) {
return false;
} else {
return true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,8 @@ public function register_hooks() {
/**
* Add our logo uploader to the Customizer.
*
* This uses the old Jetpack format. This function should only load in a context where `custom-logo` is not supported.
*
* @param object $wp_customize Customizer object.
* @uses current_theme_supports()
* @uses current_theme_supports()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,15 +18,15 @@ function jetpack_get_site_logo( $show = 'url' ) {
$logo = site_logo()->logo;

// Return false if no logo is set
if ( ! isset( $logo['id'] ) || 0 == $logo['id'] ) {
if ( empty( $logo ) ) {
return false;
}

// Return the ID if specified, otherwise return the URL by default
if ( 'id' == $show ) {
return $logo['id'];
return $logo;
} else {
return esc_url_raw( set_url_scheme( $logo['url'] ) );
return esc_url_raw( set_url_scheme( wp_get_attachment_url( $logo ) ) );
}
}

Expand Down Expand Up @@ -131,7 +131,8 @@ function jetpack_the_site_logo() {
$logo_id = get_theme_mod( 'custom_logo' );
$jetpack_logo = site_logo()->logo;

// Use WP Core logo if present and is an id (of an attachment), otherwise use Jetpack's.
// Use WP Core logo if present, otherwise use Jetpack's.
// This shouldn't really ever fire anymore with the conversion code to Core's format, but leaving it just to be safe.
if ( ! is_numeric( $logo_id ) && isset( $jetpack_logo['id'] ) ) {
$logo_id = $jetpack_logo['id'];
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ public function test_jetpack_og_get_site_icon_and_logo_url() {
$default_url = jetpack_og_get_image();

// Test Jetpack's Site Logo
update_option( 'site_logo', array( 'id' => $this->icon_id, 'url' => wp_get_attachment_url( $this->icon_id ) ) );
update_option( 'site_logo', $this->icon_id );
require_once JETPACK__PLUGIN_DIR . 'modules/theme-tools/site-logo/inc/functions.php';
require_once JETPACK__PLUGIN_DIR . 'modules/theme-tools/site-logo/inc/class-site-logo.php';

Expand Down