Assign a theme when creating a site (Multisite)

[Mista-Flo]

This is a serie of multisite focus enhancements for themes, the major one is from #13743 that add a network default theme (I used this network option in this following patch).

This ticket "Assign a theme when creating a site" was a shared idea discussed on WCUS, WC Cologne, etc.

So here a first patch that just add a select input containing the activated themes of the current network. By default, the selected theme is WP_DEFAULT_THEME constant or the new default network option for theme.

After the form is submited, we add stylesheet and template name to the $meta array, and wpmu_create_blog function makes an automatic update option with the right theme.

[Mista-Flo]

Please test this ticket with the other https://core.trac.wordpress.org/ticket/13743

[Mista-Flo]

First patch

[Mista-Flo]

Screenshot with the new select field

[flixos90]

Having had a first look at the patch, it looks good so far.

We should think about whether we actually want to allow all installed themes in the dropdown, not only those enabled network-wide. It's the network administrator who creates the site, so they should have access to it, and when they choose a theme that is not network-enabled, it could automatically be enabled for the site maybe.

Those are all just thoughts, that we should think through though.

[Mista-Flo]

Thanks for your review Felix.

I guess it makes sense, I'm okay with that.

Remark : Be carreful, my patch rely on new feature added in this patch : #13743. But it needs a refresh to use the new network_get_default_theme from this patch instead of always using
get_network_option function.

By the way, Tell me if we need to make a patch that is not linked to the other ticket mentioned above.

I can also avoid repeating myself two times with :

<?php
$stylesheet = $theme->get_stylesheet(); 
$template   = $theme->get_template(); 

I'll try to publish a new patch.

[Mista-Flo]

Better patch

[Mista-Flo]

The new patch use the network_get_default_theme function, list all themes in the select (even non network enabled ones), and automaticaly network enable themes if they're not like @flixos90 proposed.

[flixos90]

I just reviewed 39318.2.patch​. Some notes:

• Let's not rely on #13743 here. It's not necessary for this feature to work, we can use WP_DEFAULT_THEME for now. Once #13743 gets ready, we simply need to update the code.
• If the theme requested doesn't exist (see line 68), nothing should change from the current behavior. I recommend you wrap all the following code that eventually updates the $meta array into a conditional that only runs if $theme->exists(). In other words: Keep $meta the public flag only in case the requested theme doesn't exist, only set stylesheet and template if it does exist. Alternatively, we might wanna wp_die() with an error, but since providing the theme is not crucial, I'm not sure we need to do that.
• A minor suggestion: Your patch is not wrong there at all, but in line 304 and following, I think it's better readable to not use printf() and instead output HTML directly. Same for the select tag itself, it should not be echoed, but directly outputted as HTML.

[Mista-Flo]

Hello @flixos90,

Thank you for your review.

I have updated my patch to fit your suggestions.

[Mista-Flo]

Fix indentation

[flixos90]

Thanks for the update @Mista-Flo!

Just a few minor observations regarding 39318.4.patch​ (some of which I could have noticed earlier, sorry about that):

• The comment on line 70 can be removed. We're not looking at WP_DEFAULT_THEME there.
• In line 73 and 74, the equal signs should be aligned.
• The comment on line 76 can be removed.
• In line 78, the site should not be enabled on the network, but on the new site. Enabling it for the entire network would be unexpected.
• In line 297, you can remove the is_array() check, the return value of wp_get_themes() should always be an array.

[Mista-Flo]

Various fixes : Review by @flixos90

[Mista-Flo]

Hi,

Thank you,

I have uploaded a new patch.

Except about the "In line 78, the site should not be enabled on the network, but on the new site. Enabling it for the entire network would be unexpected."

Are you sure you can use a theme on a site whereas its not network enabled ? I'm not sure about this.

Replying to flixos90:

Thanks for the update @Mista-Flo!

Just a few minor observations regarding 39318.4.patch​ (some of which I could have noticed earlier, sorry about that):

• The comment on line 70 can be removed. We're not looking at WP_DEFAULT_THEME there.
• In line 73 and 74, the equal signs should be aligned.
• The comment on line 76 can be removed.
• In line 78, the site should not be enabled on the network, but on the new site. Enabling it for the entire network would be unexpected.
• In line 297, you can remove the is_array() check, the return value of wp_get_themes() should always be an array.

[flixos90]

Replying to Mista-Flo:

Except about the "In line 78, the site should not be enabled on the network, but on the new site. Enabling it for the entire network would be unexpected."

Are you sure you can use a theme on a site whereas its not network enabled ? I'm not sure about this.

Themes can also be enabled per site, via the Edit Site > Themes area. They are stored in an allowedthemes option (see ​https://github.com/WordPress/wordpress-develop/blob/master/src/wp-includes/class-wp-theme.php#L1414 for more). When you set a theme for a site, you wouldn't expect it to suddenly work for every network - it only implies you want to have it for the one site.

I think we need to implement methods WP_Theme::site_enable_theme( $stylesheets, $site_id ) and WP_Theme::site_disable_theme( $stylesheets, $site_id ). That should happen in a separate ticket though. When that is done, we can complete work on this one here. Everything else in this ticket here is ready.

[swissspidy]

That separate ticket is #43728.

[Mista-Flo]

@swissspidy @flixos90 Hey, no news here ?

[swissspidy]

Hey @Mista-Flo, sorry for the delay here. Multisite isn't really my field of expertise, so I leave this to the more knowledgable :-)

If you wanna help move this forward by addressing the last notes, that would be awesome.

The concern was this:

I think we need to implement methods WP_Theme::site_enable_theme( $stylesheets, $site_id ) and WP_Theme::site_disable_theme( $stylesheets, $site_id ). That should happen in a separate ticket though. When that is done, we can complete work on this one here. Everything else in this ticket here is ready.

These methods would be like \WP_Theme::network_enable_theme() and \WP_Theme::network_disable_theme, but just for a single site, not for all sites in a network.

If you want to help move this forward, I think it can also happen in this ticket though for the sake of easier development.