Make WordPress Core

Opened 10 years ago

Last modified 6 years ago

#30797 assigned enhancement

New function for parent theme stylesheet uri

Reported by: grapplerulrich's profile grapplerulrich Owned by: obenland's profile obenland
Milestone: Priority: normal
Severity: normal Version: 4.4
Component: Themes Keywords: needs-patch
Focuses: Cc:

Description

Request

I would like to propose a new function to fetch the parent theme. I have attached a patch.

/**
 * Retrieve URI of current parent theme stylesheet.
 *
 * The stylesheet file name is 'style.css' which is appended to {@link
 * get_template_directory_uri() stylesheet directory URI} path.
 *
 * @since 4.2.0
 *
 * @return string
 */
function get_parent_stylesheet_uri() {
	$template_dir_uri      = get_template_directory_uri();
	$parent_stylesheet_uri = $stylesheet_dir_uri . '/style.css';
	/**
	 * Filter the URI of the current parent theme stylesheet.
	 *
	 * @since 4.2.0
	 *
	 * @param string $parent_stylesheet_uri Stylesheet URI for the current parent theme.
	 * @param string $template_dir_uri      Stylesheet directory URI for the current parent theme.
	 */
	return apply_filters( 'parent_stylesheet_uri', $parent_stylesheet_uri, $template_dir_uri );
}

Background

The reason for adding this was there was a discussion how child themes should load the parent styles.css. Using @import is not best solution. The solution would be for the parent themes to load the styles for the child theme. I created a PR to do it in _s. The code looked like this but I realized this was not the best method. The problem with this method is that it is not possible to load a stylesheet between the parent theme and child theme as the order changes when you activate a child theme.

My temporary solution for a theme would be add the function _s_get_parent_stylesheet_uri() to the theme like this: https://github.com/grappler/_s/commit/3fd84b4179f727bb24a32bfd23fcef9be79033f4

By adding get_parent_stylesheet_uri() to WordPress core all themes could use this function and the code in the theme would look like this.

function _s_scripts() {
	wp_enqueue_style( '_s-style', get_parent_stylesheet_uri() );

	if ( is_child_theme() ) {
		wp_enqueue_style( '_s-child-style', get_stylesheet_uri() );
	}
}
add_action( 'wp_enqueue_scripts', '_s_scripts' );

Theme Review Requirements

On thing to take into account is that the TRT requires that themes use the function get_stylesheet_uri(). The requirement needs to be changed when this function gets added.

Attachments (5)

get_parent_stylesheet_uri.patch (1.1 KB) - added by grapplerulrich 10 years ago.
get_parent_stylesheet_uri patch
30797.patch (1.2 KB) - added by grapplerulrich 9 years ago.
30797.1.patch (1.1 KB) - added by grapplerulrich 9 years ago.
30797.diff (2.3 KB) - added by DrewAPicture 9 years ago.
Alternate approach
30797.2.patch (1.2 KB) - added by grapplerulrich 9 years ago.
Returns URI or false

Download all attachments as: .zip

Change History (26)

@grapplerulrich
10 years ago

get_parent_stylesheet_uri patch

#1 @grapplerulrich
10 years ago

I just realized I forgot to replace $stylesheet_dir_uri with $template_dir_uri.

#2 @greenshady
10 years ago

I love the idea of having a get_parent_stylesheet_uri() function. Your proposed function sounds great.

Your proposed usage could break plugins that work with the stylesheet_uri filter hook. It'd also force all existing themes to change their code to meet a new requirement that isn't necessary.

Doing it the following way keeps back-compatibility and requires no changes to existing themes and plugins (this is the method I've been recommending). It merely provides a method for loading the parent theme stylesheet if needed.

function _s_scripts() {

	if ( is_child_theme() ) {
		wp_enqueue_style( '_s-parent-style', get_parent_stylesheet_uri() );
	}

	wp_enqueue_style( '_s-style', get_stylesheet_uri() );
}
add_action( 'wp_enqueue_scripts', '_s_scripts' );

#3 @mor10
10 years ago

+1 for this. Having a standardized and consistent way to load parent styles in child themes that provides an easy method for child theme developers to inherit all, some, or none of the parent theme styles without having to know the inner workings of the parent theme is an important step towards making WordPress easier to use for all.

#4 follow-up: @grapplerulrich
10 years ago

@greenshady - Thanks!

It'd also force all existing themes to change their code to meet a new requirement that isn't necessary.

What requirement do you mean? It is optional to use the function.

One reason why I don't like the method you mentioned is that the handle switches when activating the child theme. The second reason is that it does not work when you want to include a second stylesheet after the main stylesheet like this:

function _s_scripts() {

	if ( is_child_theme() ) {
		wp_enqueue_style( '_s-parent-style', get_parent_stylesheet_uri() );
	}
	wp_enqueue_style( '_s-layout', get_template_directory_uri() . '/layouts/content-sidebar.css' );
	wp_enqueue_style( '_s-style', get_stylesheet_uri() );
}
add_action( 'wp_enqueue_scripts', '_s_scripts' );

Do you know of any plugins hooking into stylesheet_uri?

#5 in reply to: ↑ 4 @greenshady
10 years ago

Replying to grapplerulrich:

What requirement do you mean? It is optional to use the function.

The requirement you mentioned above. If it's optional to use the function, there's no need for a change in the theme review requirements and you can simply ignore everything I've written.

One reason why I don't like the method you mentioned is that the handle switches when activating the child theme.

You can use whatever handle you want with both methods. That's really up to the theme author.

The second reason is that it does not work when you want to include a second stylesheet after the main stylesheet like this:

Loading a second stylesheet works perfectly fine. I do it all the time. The code you posted works for loading all three stylesheets. You can mix them up, load them in whatever order, define dependencies, or just do it however you want.

Do you know of any plugins hooking into stylesheet_uri?

I have coded one (not in the repo) that I use on personal and client projects. I've seen at least one theme that uses the hook come through the theme review process.

Version 0, edited 10 years ago by greenshady (next)

#6 @iseulde
10 years ago

  • Version trunk deleted

#7 @obenland
9 years ago

  • Keywords has-patch needs-refresh added
  • Milestone changed from Awaiting Review to 4.4

#8 @wonderboymusic
9 years ago

  • Owner set to obenland
  • Status changed from new to assigned

#9 follow-up: @grapplerulrich
9 years ago

  • Keywords needs-refresh removed

I have uploaded a refreshed patch with a change to the function name to get_template_stylesheet_uri()

#10 in reply to: ↑ 9 ; follow-up: @obenland
9 years ago

  • Version set to trunk

Replying to grapplerulrich:

I have uploaded a refreshed patch with a change to the function name to get_template_stylesheet_uri()

Why get_template_stylesheet_uri() and not get_template_uri()?

#11 in reply to: ↑ 10 @greenshady
9 years ago

Replying to obenland:

Replying to grapplerulrich:

I have uploaded a refreshed patch with a change to the function name to get_template_stylesheet_uri()

Why get_template_stylesheet_uri() and not get_template_uri()?

Both would probably cause confusion with theme authors. It's hard enough explaining the differences between get_template_directory_uri() and get_stylesheet_directory_uri() as it is.

Might this be a good opportunity to break away from the standard template vs. stylesheet terminology?

I'd almost suggest get_parent_stylesheet_uri(), but this doesn't necessarily reflect a parent theme.

Maybe get_template_style_uri()? That wouldn't be consistent with get_stylesheet_uri() though.

At least get_template_uri() would be consistent with the other function names, even if it seems like it's referencing the directory URI rather than the stylesheet.

Hmph..I don't like any of the options.

#12 @grapplerulrich
9 years ago

Why get_template_stylesheet_uri() and not get_template_uri()?

That would make sense. I was just thinking how we could make it clear that it is linking to the stylesheet.

For reference all of the function names.

get_template_directory()
get_template_directory_uri()
get_stylesheet_directory()
get_stylesheet_directory_uri()
get_stylesheet_uri()

#13 @DrewAPicture
9 years ago

Citing @greenshady's comment:11 on breaking away from template vs stylesheet, I've added 30797.diff.

It introduces an argument to get_stylesheet_uri() to optionally retrieve the parent stylesheet:

<?php
function _s_scripts() {

        if ( is_child_theme() ) {
                wp_enqueue_style( '_s-parent-style', get_stylesheet_uri( 'parent' ) );
        }

        wp_enqueue_style( '_s-style', get_stylesheet_uri() );
}
add_action( 'wp_enqueue_scripts', '_s_scripts' );

If $type is 'parent' pull via get_template_directory_uri(). If the parent theme exists, you get the parent stylesheet, otherwise it falls back as expected.

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

@DrewAPicture
9 years ago

Alternate approach

#14 @grapplerulrich
9 years ago

I like the alternative approach! :)

This ticket was mentioned in Slack in #core by sergey. View the logs.


9 years ago

#16 in reply to: ↑ 15 @DrewAPicture
9 years ago

  • Milestone changed from 4.4 to Future Release

Replying to slackbot:

This ticket was mentioned in Slack in #core by sergey. View the logs.

Punting to Future Release as we don't yet have a consensus on the best way forward here.

This ticket was mentioned in Slack in #core by greenshady. View the logs.


9 years ago

#18 in reply to: ↑ 17 @DrewAPicture
9 years ago

  • Keywords needs-patch added; has-patch removed

Replying to slackbot:

This ticket was mentioned in Slack in #core by greenshady. View the logs.

'm on board with the original proposed naming of get_parent_stylesheet_uri() with the caveat that it doesn't fall back to something else. The current precedent set by get_template_directory() et al really doesn't help with reducing confusion. The function should pull the parent theme stylesheet URI or nothing.

Now we just need a patch.

@grapplerulrich
9 years ago

Returns URI or false

#19 follow-up: @grapplerulrich
9 years ago

I have uploaded a new patch. I went with returning false when a child theme is not active is because of wp_enqueue_style(). The URI is not added if it returns false.

Code snippet from wp_enqueue_style()

if ( $src ) {
        $_handle = explode('?', $handle);
        $wp_styles->add( $_handle[0], $src, $deps, $ver, $media );
}

I am not sure how I would implement the function in a theme if it did not return the current theme URI if a child theme was not active. The code that I previously would have been:

wp_enqueue_style( '_s-style', get_template_stylesheet_uri() );
if ( is_child_theme() ) {
        wp_enqueue_style( '_s-child-style', get_stylesheet_uri() );
}

#20 in reply to: ↑ 19 @SergeyBiryukov
9 years ago

Replying to grapplerulrich:

I have uploaded a new patch. I went with returning false when a child theme is not active is because of wp_enqueue_style(). The URI is not added if it returns false.

There's no strict comparison, so an empty string would be the same as false in that context.

I am not sure how I would implement the function in a theme if it did not return the current theme URI if a child theme was not active.

The example from comment:2 should work.

#21 @DrewAPicture
9 years ago

#34666 was marked as a duplicate.

Note: See TracTickets for help on using tickets.