WordPress.org

Make WordPress Core

Opened 22 months ago

Last modified 3 months ago

#21256 new feature request

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

Reported by: ramiy Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version: 3.4.1
Component: Themes Keywords: dev-feedback 2nd-opinion
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 (47)

comment:1 follow-up: ocean9021 months 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.

comment:2 johnbillion21 months ago

  • Cc johnbillion added

comment:3 cais21 months 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.

comment:4 in reply to: ↑ 1 ; follow-up: ramiy21 months 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 earier.

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.

Version 0, edited 21 months ago by ramiy (next)

comment:5 sabreuse21 months ago

  • Cc sabreuse@… added

comment:6 obenland21 months ago

  • Cc konstantin@… added

comment:7 lancewillett21 months ago

  • Cc lancewillett added

comment:8 ramiy15 months 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.

comment:9 DrewAPicture15 months ago

+1 for comment:8 from @ramly

comment:10 ramiy15 months ago

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

comment:11 ramiy15 months 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;

comment:12 kovshenin15 months ago

I like this.

comment:13 ramiy15 months 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?

comment:14 greenshady15 months ago

  • Cc justin@… added

comment:15 iamtakashi15 months ago

  • Cc takashi@… added

comment:16 jeherve14 months ago

  • Cc jeremy+wp@… added

comment:17 janw.oostendorp14 months ago

  • Cc janw.oostendorp@… added

comment:18 markmcwilliams14 months ago

  • Cc mark@… added

I really like this!

comment:19 nacin14 months ago

#17130 was marked as a duplicate.

comment:20 Mamaduka14 months ago

  • Cc georgemamadashvili@… added

comment:21 Jayjdk14 months ago

  • Cc kontakt@… added

comment:22 MikeHansenMe14 months ago

  • Cc mdhansen@… added

comment:23 follow-up: maor14 months 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.

comment:24 in reply to: ↑ 23 DrewAPicture14 months 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.

comment:25 follow-ups: nacin14 months 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.

comment:26 in reply to: ↑ 25 ramiy14 months 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 14 months ago by ramiy (previous) (diff)

comment:27 in reply to: ↑ 25 maor14 months 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.

comment:28 in reply to: ↑ 4 ramiy14 months 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' );

comment:29 philiparthurmoore13 months ago

  • Cc philip@… added

comment:30 follow-up: cliffascent13 months 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()

comment:31 follow-up: studiograsshopper12 months 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?)

comment:32 in reply to: ↑ 31 ; follow-up: DrewAPicture12 months 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.

comment:33 wycks12 months 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

comment:34 sunnyratilal11 months ago

  • Cc sunnyratilal5@… added

comment:35 alex-ye11 months ago

  • Cc nashwan.doaqan@… added

comment:36 in reply to: ↑ 32 ; follow-up: jcastaneda9 months 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

comment:37 in reply to: ↑ 36 helen9 months 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.

comment:38 buffler8 months ago

  • Cc jeremy.buller@… added

comment:39 Mat Lipe8 months ago

  • Cc Mat Lipe added

comment:40 emzo7 months ago

  • Cc wordpress@… added

comment:41 in reply to: ↑ 30 afercia7 months ago

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

comment:42 lkraav7 months ago

  • Cc leho@… added

comment:43 in reply to: ↑ description madalinignisca5 months ago

+1 for this feature

comment:44 Frank Klein3 months ago

  • Cc contact@… added

comment:45 chrisvanpatten3 months ago

  • Cc chris+wp@… added

comment:46 itsabhik3 months ago

What about adding % support in $content_width?

Like;

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

Also, +1 for the add_theme_support.

comment:47 chrisvanpatten3 months 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.

Note: See TracTickets for help on using tickets.