Make WordPress Core

Opened 12 years ago

Last modified 6 years ago

#21256 assigned feature request

New theme feature - add_theme_support( 'content-width', $defaults )

Reported by: ramiy's profile ramiy Owned by: chriscct7's profile chriscct7
Milestone: Awaiting Review Priority: normal
Severity: normal Version: 3.4.1
Component: Themes Keywords: dev-feedback needs-patch
Focuses: Cc:

Description

Themes use $content_width variable to set the content area width, they use:

if ( ! isset( $content_width ) ) 
	$content_width = 500; 

This method has two flaws, it's not flexible and it does not support different sizes for different post-types.

WordPress has to make the content-width to be a builtin theme feature using add_theme_support(), and make it more flexible and easy to update. I want to update this value using the Theme Customizer rather editing the function.php file.

The code needs to be easy to set and to support CPT, some thing like this:

$defaults = array(
	'post'       => '500',
	'page'       => '500',
	'attachment' => '650',
	'artist'     => '300',
	'movie'      => '400'
);
add_theme_support( 'content-width', $defaults );

Just an idea for 3.5.

Change History (54)

#1 follow-up: @ocean90
12 years ago

I really like the idea of using add_theme_support here.

This method has two flaws, it's not flexible and it does not support different sizes for different post-types.

That's not right, you could can the variable on the fly, see Twenty Twelve.

#2 @johnbillion
12 years ago

  • Cc johnbillion added

#3 @cais
12 years ago

  • Cc edward.caissie@… added

Great idea! Especially since I'm currently having to write some convoluted conditionals to do what I want in a new theme.

#4 in reply to: ↑ 1 ; follow-up: @ramiy
12 years ago

Replying to ocean90:

You are right, Twenty Twelve has a very nice workaround for full-width page-template:

if ( ! isset( $content_width ) )
	$content_width = 625;

function twentytwelve_content_width() {
	if ( is_page_template( 'full-width-page.php' ) || is_attachment() ) {
		global $content_width;
		$content_width = 960;
	}
}
add_action( 'template_redirect', 'twentytwelve_content_width' );

I like this approach, but using conditionals to set different sizes for different post-types requires many lines of code. with $defaults array it's easier.

And remember that other themes do it in different ways. There is no standard way to do this. If we will make it a core-theme-feature, all themes will use it the same way.

Last edited 12 years ago by ramiy (previous) (diff)

#5 @sabreuse
12 years ago

  • Cc sabreuse@… added

#6 @obenland
12 years ago

  • Cc konstantin@… added

#7 @lancewillett
12 years ago

  • Cc lancewillett added

#8 @ramiy
12 years ago

As a first step towards this method we can do it much simpler.

Instead of:

if ( ! isset( $content_width ) ) 
	$content_width = 500;

We can do this:

add_theme_support( 'content-width', 500 );

In the future, if we would like to extend this feature, we can use is_array() for different post-types or page-template.

#9 @DrewAPicture
12 years ago

+1 for comment:8 from @ramly

#10 @ramiy
12 years ago

Do not want to jump the gun, but i created a codex page for Content Width theme-feature.

#11 @ramiy
12 years ago

For backwards compatibility with older versions, theme authors can use the following code:

global $wp_version;

if ( version_compare( $wp_version, '3.6', '>=' ) ) :

	add_theme_support( 'content-width', 500 );

else :

	if ( ! isset( $content_width ) ) 
		$content_width = 500;

endif;

#12 @kovshenin
12 years ago

I like this.

#13 @ramiy
12 years ago

Question for core devs:

add_theme_support() function hooked into after_setup_theme action, wich is the first action available after functions.php file is loaded.

What effect would it have on WordPress if we move $content_width into after_setup_theme, if any?

#14 @greenshady
12 years ago

  • Cc justin@… added

#15 @iamtakashi
12 years ago

  • Cc takashi@… added

#16 @jeherve
12 years ago

  • Cc jeremy+wp@… added

#17 @janw.oostendorp
12 years ago

  • Cc janw.oostendorp@… added

#18 @markmcwilliams
12 years ago

  • Cc mark@… added

I really like this!

#19 @nacin
12 years ago

#17130 was marked as a duplicate.

#20 @Mamaduka
12 years ago

  • Cc georgemamadashvili@… added

#21 @Jayjdk
12 years ago

  • Cc kontakt@… added

#22 @MikeHansenMe
12 years ago

  • Cc mdhansen@… added

#23 follow-up: @maor
12 years ago

  • Cc maorhaz@… added

I love this approach.

I also tend to think that it's more of a theme feature. Having it as a global variable is quite risky, since plugins and themes can override this variable by mistake.

#24 in reply to: ↑ 23 @DrewAPicture
12 years ago

Replying to maor:

I also tend to think that it's more of a theme feature. Having it as a global variable is quite risky, since plugins and themes can override this variable by mistake.

I think we'd have to keep the global for back-compat anyway.

Not sure I'm completely onboard with doing a defaults array per-post type as suggested above though. I'd be tempted to roll this into an add_theme_support() implementation then still have themes filter the value that goes in based on their own conditions.

#25 follow-ups: @nacin
12 years ago

I don't know how beneficial this is when there is an increasing desire to modify content_width based on certain contexts, like a particular page template.

I'm actually a bit concerned about modifying content_width on the fly, because of the potential side effects it has. It is good for only three things:

  • Embed width — wp_embed_defaults()
  • What images are scaled down to when inserted into the editor — image_constrain_size_for_editor()
  • What the fullscreen editor uses as a default width — WP_Editor's wp_fullscreen_html()

Only the first two are important. The problem is that both are used in an admin context, so Twenty Twelve's hook into template_redirect accomplishes, as far as I can tell, nothing. (Unless the embed isn't cached by the time the front page is loaded.) Same for Twenty Thirteen.

Turning this into theme support seems okay, but it seems like there needs to be a getter function (a la #17130) plus a filter — but done in a way that provides context and actually works.

To toss out another use case, Twenty Thirteen is seeing a need to have a "content width" being different from the "embed width". Realistically, this isn't a "content width" but an "embed width" setting, anyway — CSS is what has an effect on the content itself, not this global.

It just seems the whole idea of $content_width needs a re-think.

#26 in reply to: ↑ 25 @ramiy
12 years ago

Replying to nacin:

Twenty Thirteen is seeing a need to have a "content width" being different from the "embed width". Realistically, this isn't a "content width" but an "embed width" setting, anyway — CSS is what has an effect on the content itself, not this global.

It just seems the whole idea of $content_width needs a re-think.

Nacin, i think we should rename this from "content width" to "content max width" or even better to "max width". The terminology will represent the usage - set "max width" not only for "content width" but also for "embed width".

And the CSS effects only the theme (on the front end), not the dashboard. If some plugin will try to add something to the content, it won't know what is the max-width defined in the CSS.

Last edited 12 years ago by ramiy (previous) (diff)

#27 in reply to: ↑ 25 @maor
12 years ago

Replying to nacin:

It just seems the whole idea of $content_width needs a re-think.

I can totally relate to that.

Like it was noted before, I also feel that the "issue" here is that content_width lacks context. While I don't like the way of changing content_width on the fly, there is really not an easier way to do that other than hooking into template_redirect (at least for the time being).

Just throwing out an idea: what if it was possible to set content_width for any template file (either a "template" or theme file such as page.php, single.php, etc), by having this new line in the file headers.

<?php
/**
 * Template Name: Sidebar Template
 * Description: A Page Template that adds a sidebar to pages
 * Content Width: 550
 *
 * @package WordPress
 * @subpackage Twenty_Eleven
 * @since Twenty Eleven 1.0
 */

It's not perfect. What might be good about this is that we do have context. The downside is when a dashboard operation is taking place -- there is no easy way to know which content-width setting is the right one.

#28 in reply to: ↑ 4 @ramiy
12 years ago

Ok, let's re-think $content_width, and how we use it?

Dashboard:

  • Fullscreen editor default width.
  • Scaled down images in the editor.

Themes:

  • Set maximum allowed width for Embeds and Images in the content.
  • Use conditionals for different pages.

I analyzed how different theme's use $content_width.

In my themes, i use conditionals for post-types.

Twenty Twelve uses conditionals for page-templates (wide screen):

if ( ! isset( $content_width ) )
	$content_width = 625;

function twentytwelve_content_width() {
	if ( is_page_template( 'full-width-page.php' ) || is_attachment() ) {
		global $content_width;
		$content_width = 960;
	}
}
add_action( 'template_redirect', 'twentytwelve_content_width' );

Twenty Thirteen uses conditionals for post-formats (videos & images):

if ( ! isset( $content_width ) )
	$content_width = 604;

function twentythirteen_content_width() {
	if ( has_post_format( 'image' ) || has_post_format( 'video' ) || is_attachment() ) {
		global $content_width;
		$content_width = 724;
	}
}
add_action( 'template_redirect', 'twentythirteen_content_width' );

#29 @philiparthurmoore
12 years ago

  • Cc philip@… added

#30 follow-up: @cliffascent
12 years ago

As far as I can tell, the content_width only effects embedded media. I use the following conditional content_width in my theme, and it changes the content_width of embedded videos based on my sidebar layout as expected, but I didn't notice a difference with adding images into posts or the image sizes created for new images. My only problem with this is my theme is responsive, so when the content_width is 700 that will fit the full size, but in smaller screens the video is cut off. I plan to make some JavaScript that will hopefully fix the responsive aspect. I wish embedded media was just 100% and content_width didn't exist, since I'm trying to remain responsive.

if ( ! function_exists( 'ascension_get_content_width' ) ) :
function ascension_get_content_width() {
	global $content_width;
	$theme_options = ascension_get_theme_options();
	
	// Get the proper page type to append to the sidebar layout option.
	if ( is_home() )
		$sidebar_layout_type = '_home';
	elseif ( is_archive() )
		$sidebar_layout_type = '_archive';
	elseif ( is_singular() ) {
		if ( is_page() )
			$sidebar_layout_type = '_page';
		else
			$sidebar_layout_type = '_singular';
	}
	elseif ( is_search() )
		$sidebar_layout_type = '_search';
	elseif ( is_404() )
		$sidebar_layout_type = '_404';
	else
		$sidebar_layout_type = '';
	
	// Width chosen is the content area width minus any padding or borders.
	if ( $theme_options['sidebar_layout' . $sidebar_layout_type] == 'Left Sidebar' )
		$content_width = 700;
	elseif ( $theme_options['sidebar_layout' . $sidebar_layout_type] == 'Right Sidebar' )
		$content_width = 700;
	elseif ( $theme_options['sidebar_layout' . $sidebar_layout_type] == 'Left and Right Sidebars' )
		$content_width = 560;
	elseif ( $theme_options['sidebar_layout' . $sidebar_layout_type] == 'Double Left Sidebars' )
		$content_width = 560;
	elseif ( $theme_options['sidebar_layout' . $sidebar_layout_type] == 'Double Right Sidebars' )
		$content_width = 560;
	elseif ( $theme_options['sidebar_layout' . $sidebar_layout_type] == 'No Sidebars' )
		$content_width = 940;
}
endif; // End ascension_get_content_width()

#31 follow-up: @studiograsshopper
12 years ago

This seems to me to be a lot of hoops to jump through to deal with something which shouldn't be dealt with by PHP/HTML in the first place. Why not ditch $content_width altogether and let CSS do the work?
(Or am I missing something obvious?)

#32 in reply to: ↑ 31 ; follow-up: @DrewAPicture
12 years ago

Replying to studiograsshopper:

This seems to me to be a lot of hoops to jump through to deal with something which shouldn't be dealt with by PHP/HTML in the first place. Why not ditch $content_width altogether and let CSS do the work?
(Or am I missing something obvious?)

$content_width also controls the width of the content in the post editor so you get a realistic preview of what it will look like.

#33 @wycks
12 years ago

  • Cc bob.ellison@… added

On many sites the use of the $content_width is the largest reason for slow page loading due to how it resizes images via HTML. Think about all the sites using WordPress and the enormous amount of bandwidth pollution it causes.

Using it for embeds and editor previews seems to be fine since this has little to no effect on the performance of a site, using it to resize images on the other hand does effect the user experience.

The only thing I can think of would be to set the max width for images into an actual real image size, something like max-post-width instead of just the default 3 (which is exactly what I do in many themes).

Discussion on WPSE

#34 @sunnyratilal
12 years ago

  • Cc sunnyratilal5@… added

#35 @alex-ye
12 years ago

  • Cc nashwan.doaqan@… added

#36 in reply to: ↑ 32 ; follow-up: @jcastaneda
11 years ago

  • Cc jomcastaneda@… added

Replying to DrewAPicture:

Replying to studiograsshopper:

This seems to me to be a lot of hoops to jump through to deal with something which shouldn't be dealt with by PHP/HTML in the first place. Why not ditch $content_width altogether and let CSS do the work?
(Or am I missing something obvious?)

$content_width also controls the width of the content in the post editor so you get a realistic preview of what it will look like.

From what I've experienced it is actually the editor stylesheet that handles that and not $content_width

#37 in reply to: ↑ 36 @helen
11 years ago

Replying to jcastaneda:

From what I've experienced it is actually the editor stylesheet that handles that and not $content_width

Yes and no - it does affect the width of the fullscreen/distraction free editor.

#38 @buffler
11 years ago

  • Cc jeremy.buller@… added

#39 @Mat Lipe
11 years ago

  • Cc Mat Lipe added

#40 @emzo
11 years ago

  • Cc wordpress@… added

#41 in reply to: ↑ 30 @afercia
11 years ago

+1 for "I wish embedded media was just 100% and content_width didn't exist" :-)

#42 @lkraav
11 years ago

  • Cc leho@… added

#43 in reply to: ↑ description @madalinignisca
11 years ago

+1 for this feature

#44 @Frank Klein
11 years ago

  • Cc contact@… added

#45 @chrisvanpatten
11 years ago

  • Cc chris+wp@… added

#46 @itsabhik
11 years ago

What about adding % support in $content_width?

Like;

if ( ! isset( $content_width ) ) 
	$content_width = 70%;

Also, +1 for the add_theme_support.

#47 @chrisvanpatten
11 years ago

Support for % widths would be nice, but that introduces a lot of complications with embeds, because you'd have to recalculate the height every time the browser width changes so the embed scaled correctly.

As I think about it more, I'm starting to agree with the folks who suggest ditching $content_width entirely. On responsive sites—which are becoming more and more the norm—$content_width is pretty useless. Yes, it affects the editor width as well, but on a responsive site, you're essentially making a promise to the end user that the content will look a certain way, when in fact there's no way to guarantee that.

I know it probably won't be removed, but I'm not sure it's worth migrating to the add_theme_support() style either. Seems like too much work for too little gain, especially in the age of responsiveness.

#49 follow-up: @wpsuperflex
10 years ago

$content_width should really be "$possible_content_widths" that are made available to CSS. The idea of setting a content width (even if it's based on testing for things like presence of sidebars) is flawed because it assumes the web page can only have one possible width. Such ideas are very dated in the modern variable-screen world. Any reimplementation of $content_width should take responsive design into consideration.

#50 in reply to: ↑ 49 @iCaspar
10 years ago

+1 for this!
It seems like content-width ought to function in a way similar to thumbnail sizes. Allow themes use a default array and add a filter with custom sizes via add-theme-support(). Sizing in the editor (as @chrisvanpatten says) is generally useless in a responsive world, but it could still be available if a theme/plugin really needs it for those who aren't ready to give it up.

This ticket was mentioned in Slack in #themereview by karmatosed. View the logs.


10 years ago

#52 @chriscct7
9 years ago

  • Keywords needs-patch added; 2nd-opinion removed
  • Owner set to chriscct7
  • Status changed from new to assigned

#53 @markcallen
7 years ago

I'm not so sure about add_theme_support here, any defined list of arguments is entirely subjective and could be based on any number of factors

  • Screen size (unknown at the time of generation anyway)
  • Post type, as inferred by the OP
  • Template
  • Post format
  • Taxonomy, date, author pages
  • Something I haven't thought of

It's inappropriate for core to try and make assumptions about any possible layout conditions and provide a pre-determined list of arguments to decide. IT's just creating future back-compat issues IMO.

This is why a simple filter value makes most sense to me. e.g.

apply_filters( 'content_width', $content_width );

Theme devs can then add filters based on their own requirements to get the right value out. It removes the need for hooking into template_redirect, or any other workaround because it can be hooked directly and appropriately.

The definition in themes then becomes the default value.

Ultimately, the continued existence of $content_width should really be challenged. Any way you look at it, it shouldn't have a long term future.

Last edited 7 years ago by markcallen (previous) (diff)

#54 @Horttcore
6 years ago

It would also be great if the Gutenberg editor screen would respect the $content_width variable.

Note: See TracTickets for help on using tickets.