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

Child theme styles get not loaded #682

Closed
nielslange opened this issue Oct 1, 2019 · 20 comments · Fixed by #683
Labels

Comments

@nielslange
Copy link
Collaborator

@nielslange nielslange commented Oct 1, 2019

Steps to reproduce

  1. Head over to /wp-content/themes/
  2. Create a child theme folder, e.g. twentytwenty-child
  3. Create the file style.css
  4. Add /* Template: twentytwenty */ to style.css
  5. Activate the child theme

Expected behaviour

The child styles are loaded:

<link rel='stylesheet' id='twentytwenty-style-css' href='http://wp.test/wp-content/themes/twentytwenty-child/style.css?ver=5.2.3' type='text/css' media='all' />

Current behaviour

The parent styles are loaded while the child styles are not loaded:

<link rel='stylesheet' id='twentytwenty-style-css' href='http://wp.test/wp-content/themes/twentytwenty/style.css?ver=5.2.3' type='text/css' media='all' />

@nielslange nielslange added the bug label Oct 1, 2019
nielslange added a commit to nielslange/twentytwenty that referenced this issue Oct 1, 2019
@nielslange nielslange added the Has PR label Oct 1, 2019
@acosmin

This comment has been minimized.

Copy link
Contributor

@acosmin acosmin commented Oct 1, 2019

@nielslange This just doesn't load the parent's stylesheet which is needed.

Based on docs you still need to enqueue it: https://developer.wordpress.org/themes/advanced-topics/child-themes/#3-enqueue-stylesheet

@nielslange

This comment has been minimized.

Copy link
Collaborator Author

@nielslange nielslange commented Oct 1, 2019

@nielslange This just doesn't load the parent's stylesheet which is needed.

@acosmin As mentioned in the description of this issue the child theme styles get not loaded. Thus, instead of /wp-content/themes/twentytwenty-child/style.css?ver=5.2.3 the URL /wp-content/themes/twentytwenty/style.css?ver=5.2.3 gets loaded when having the chile theme activated.

Have you created a child theme yet and do you see the URL /wp-content/themes/twentytwenty-child/style.css?ver=5.2.3 in the source code?

Based on docs you still need to enqueue it: https://developer.wordpress.org/themes/advanced-topics/child-themes/#3-enqueue-stylesheet

I'm aware of that. However, in this case, the parent style gets loaded for both the parent and the child theme, which should not be the case. 😉

andersnoren added a commit that referenced this issue Oct 1, 2019
@joyously

This comment has been minimized.

Copy link

@joyously joyously commented Oct 1, 2019

Your "Expected behavior" isn't actually expected.
Child themes always need to look at the parent to see what to load.
But the parent can be written in such a way as to load both, so the child does not have to.

Edit:

However, in this case, the parent style gets loaded for both the parent and the child theme, which should not be the case.

Yes, the parent style should always be loaded. If the child theme does not want the parent style, then the child can handle that, but it's an edge case.

@nielslange

This comment has been minimized.

Copy link
Collaborator Author

@nielslange nielslange commented Oct 1, 2019

@joyously I’m afraid you misunderstood the issue here. The problem was that the child styles did not get loaded initially. Of course, the parent styles need to be loaded as well, but that’s not default functionality but requires enqueueing the parent styles in the child theme’s ’function.php’ as described on https://developer.wordpress.org/themes/advanced-topics/child-themes/#3-enqueue-stylesheet

@joyously

This comment has been minimized.

Copy link

@joyously joyously commented Oct 1, 2019

That is totally backwards, and we need to amend that Handbook page!
The parent should load the parent styles, using its version number. Optionally, it can load the child styles, using the child version number.
The child should never have to load the parent styles! If the parent does not load the child styles, the child should load it.

@nielslange

This comment has been minimized.

Copy link
Collaborator Author

@nielslange nielslange commented Oct 1, 2019

I’m afraid I don’t know what point you’re trying to make here. Isn’t it wrong that the child theme loads the parent styles instead of the child styles? How’s the child theme suppose to work if the child theme styles are not loaded?

@andersnoren, @carolinan, @pattonwebz, @ianbelanger79 and @ntwb Can we get your thoughts on this as well?

@joyously

This comment has been minimized.

Copy link

@joyously joyously commented Oct 1, 2019

It is wrong (in my opinion) that the child theme loads the parent styles. I was just saying that the original "Expected Behavior" listed here said that the child styles would be loaded. I would not expect that.

@nielslange

This comment has been minimized.

Copy link
Collaborator Author

@nielslange nielslange commented Oct 1, 2019

It is wrong (in my opinion) that the child theme loads the parent styles. I was just saying that the original "Expected Behavior" listed here said that the child styles would be loaded. I would not expect that.

That’s exactly what I wrote in Expected Behavior, isn’t it? Take a closer look at the style that actually gets loaded. I expect that within a child theme the child theme styles are loaded instead of the parent styles.

@joyously

This comment has been minimized.

Copy link

@joyously joyously commented Oct 1, 2019

No. You expected that the parent would load the child styles. I would not expect that, although it is optional, and it is how I coded my theme.
I expect that the parent always loads the parent styles. Optionally, the parent loads the child styles.
Depending on what the parent chooses for child styles, the child loads its own or not.
The child should never have to load the parent style, but this has been pushed as recommended in the handbook.
(I initiated a change to the handbook a little while ago.)

@nielslange

This comment has been minimized.

Copy link
Collaborator Author

@nielslange nielslange commented Oct 1, 2019

Nope, that’s not correct, @joyously, and it’s not what I wrote. Feel free to read the description of this issue carefully. 😉

@joyously

This comment has been minimized.

Copy link

@joyously joyously commented Oct 1, 2019

I did read it carefully. You and I have different expectations.

@nielslange

This comment has been minimized.

Copy link
Collaborator Author

@nielslange nielslange commented Oct 1, 2019

At least, the issue is fixed and child themes can be created as expected. That’s all what counts, doesn’t it?

@joyously

This comment has been minimized.

Copy link

@joyously joyously commented Oct 1, 2019

No, it made it the worst way instead of how it should be.
When the parent uses get_stylesheet_uri() for the parent styles, with the parent version number, and the parent dependencies, then the child gets the wrong handle, the wrong version, and the wrong dependencies.

@nielslange

This comment has been minimized.

Copy link
Collaborator Author

@nielslange nielslange commented Oct 1, 2019

How about you gonna create a PR to fix this issue then, if the current PR made it worse. 🤷‍♂️

@joyously

This comment has been minimized.

Copy link

@joyously joyously commented Oct 1, 2019

It wasn't broken. This PR should just be reverted.
If the parent theme wants to load the child styles, that should be in addition to the parent styles, not in place of it.
At least this one uses the dynamic version variable, so that part is okay. But doing it this way means the child has to load the parent styles, which it should never have to do.

@nielslange

This comment has been minimized.

Copy link
Collaborator Author

@nielslange nielslange commented Oct 1, 2019

We’re going in circles, @joyously. What you’re saying is not how child themes are documented. That’s all I can say.

@joyously

This comment has been minimized.

Copy link

@joyously joyously commented Oct 1, 2019

Yes, it's issues like this that are supposed to help improve our documentation and our code and our standard practices.
We should do it the best way possible, not just follow the wrong documentation.
I still think this PR broke the theme rather than fixed it. @andersnoren

@joyously

This comment has been minimized.

Copy link

@joyously joyously commented Oct 1, 2019

This is how I code it in my theme, which doesn't have dependencies for the parent, but those are easily added, like already coded in this theme:

	$theme = wp_get_theme();
	$version = $theme->parent() ? $theme->parent()->get( 'Version' ) : $theme->get( 'Version' );
	wp_enqueue_style( 'twenty8teen-style', get_template_directory_uri() .
		'/style.css', array(), $version );
	if ( is_child_theme() ) {
		wp_enqueue_style( get_stylesheet() . '-style', get_stylesheet_uri(),
			array( 'twenty8teen-style' ), $theme->get( 'Version' ) );
	}
@mtoensing

This comment has been minimized.

Copy link

@mtoensing mtoensing commented Oct 2, 2019

Why is this issue closed? I came here to open the same bug as well. If you create the minimal child theme possible with twentytwenty the child themes css never gets loaded.

@mtoensing

This comment has been minimized.

Copy link

@mtoensing mtoensing commented Oct 2, 2019

Ah, it got fixed in #683 my bad.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.