-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
Move $content_width declaration inside _s_setup() #100
Comments
|
I looked into this a bit last night and tested with a bunch of plugins. What I discovered is that there is practically no difference between setting
I looked through core and there are no actions fired between the time where functions.php is loaded and when the action is fired. Personally, I like the idea of moving the setting of this variable into a function hooked into |
Can anyone think of a reason not to do it? Yes. Through many bugs with plugins and much trial and error we've found that the best approach is defining the first The base definition won't work if it fires on actions like Other secondary content_width calculations (based on widget areas, page template, etc) can be in a hooked function, however — that's fine. |
The base definition won't work if it fires on actions like after_setup_theme or template_redirect because it will happen too late for plugins like Post by Email and Tiled Galleries (both in use on WP.com and both are in or coming soon to Jetpack). Those plugins are going to run into many issues with themes adding I can't think of any reason a plugin would assume the It definitely shouldn't happen on |
I think this ticket should be closed for the following reasons:
|
Agreed. Closing it. |
Guys, i created documentation on the codex for $content_width: And check out the tract ticket #21256 (http://core.trac.wordpress.org/ticket/21256), may be relevant to this discussion. |
Underscores doing something the right way - such as defining I disagree with the argument that globals should be set as early as possible. They should be set where they would logically be expected to be set. In the case of a Theme-defined global, that would be Lots of Theme developers are now using Underscores as the basis for the Themes that they submit to the Theme directory - and they're all having to modify it to move the Do I need to go submit patches for all the Twenty-X Themes, so that Underscores can then conform to the core-bundled Theme behavior? And @raimy +1 to your patch! |
I agree with comments on the core Trac ticket that content width needs a complete re-think. About this specific change: I still feel we should keep the current code as-is, in both _s and core themes. It's a pragmatic approach, working around things that are broken (like certain plugins). It speaks to one of the most central WordPress values of backward compatibility. |
Update to WordPress 4.0
On one of the articles in theThemeShaper WordPress Theme Tutorial: 2nd Edition Series, which uses code based on _s, it's been suggested in the comments that $content_width should be moved inside _s_setup().
Here is the reason given in the comments (by Justin Tadlock):
(I'm aware that _s is not intended to be a parent theme, though.)
Chip Bennett commented:
Opinions?
The text was updated successfully, but these errors were encountered: