WordPress.org

Make WordPress Core

Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#15078 closed defect (bug) (fixed)

Custom Loops in Twenty Ten

Reported by: koopersmith Owned by:
Milestone: 3.1 Priority: normal
Severity: normal Version: 3.0.1
Component: Themes Keywords: has-patch dev-reviewed
Focuses: Cc:

Description

In Twenty Ten, we use get_template_part to allow users to override loop.php.
This is a beautiful thing.

However, when files in Twenty Ten use anything other than the default loop, they don't use get_template_part('loop','custom') and loop-custom.php. Instead, they just hardcode the loop into the file.

We should fix that.

Attachments (1)

15078.diff (20.6 KB) - added by koopersmith 5 years ago.

Download all attachments as: .zip

Change History (13)

@koopersmith5 years ago

comment:1 @iandstewart5 years ago

  • Cc ian@… added

comment:2 @nacin5 years ago

Big plus one from me.

comment:3 @demetris5 years ago

  • Cc dkikizas@… added

If I understand and remember correctly (I will have to look at the code), the second argument of get_template_part only makes sense if it offers an option of finer control.

To give an example, in Daryl’s patch I would use:

get_template_part('loop-page', '1col');

for the single-column page templage, and just

get_template_part('loop-page');

for the main page template.

Then users could make a loop-page-1col.php file to change the loop only for the single-column template.

(By the way, why do single-view templates need a loop? Doesn’t the_post() set up everything needed there?)

comment:4 @nacin5 years ago

Demetris -- I see your point as it relates to differentiating the regular page from the single-column page, but beyond that, I like the consistency of 'loop' and then the context argument. It provides for easy overriding in a child theme, or in this case, we're actually overriding the loop directly in the parent theme, too. Also, if you wanted to check if you were in a page template, there's a function for that -- is_page_template().

With regards to why we need a proper loop, see #13279.

comment:5 @westi5 years ago

  • Keywords dev-reviewed added

This is completely the intention of get_template_part() usage in TwentyTen

And takes it to the next level!

We missed some :-(

comment:6 @nacin5 years ago

(In [15763]) svn add attachment.php. see #15078, r15762.

comment:7 @nacin5 years ago

  • Resolution set to fixed
  • Status changed from new to closed

I was so close to full preservation of history, but forgot to add attachment.php. Aw well :-)

Looks like [15762] never made it here. Closing as fixed and will backport to the 3.0 branch in preparation for Twenty Ten 1.2 (note @since's) this weekend.

comment:8 follow-up: @demetris5 years ago

@nacin

As I see it, a main/parent theme should never use both args of get_template_part to specify its own template parts.

First, there is no reason to: A theme can, equally well, specify any file it wants by using only the first argument.

Second, there is reason not to: By doing so, it denies child themes the option to do something more specific for a template. So, it cancels the granularity and flexibility that the second arg of get_template_part gives us.

Why have a second argument in the first place if parent themes use it up to do what they can very well do by using only one argument?

Given these two points, the options for organizing the loop code of Twenty Ten are, I think, two:

First option:

Drop all loop code in one loop.php file, and then give options to child themes by including loop.php like this:

get_template_part('loop', 'single');

get_template_part('loop', 'page');

get_template_part('loop', 'home');

get_template_part('loop', 'category');

etc. etc.

That, of course, would mean a large loop file with complex, difficult to follow, flow control. (And maybe with a performance penalty too?)

Second option, not very backwards compatible maybe at this moment:

get_template_part('loop-archive', 'author');

get_template_part('loop-archive', 'category');

etc. etc.

get_template_part('loop-page', 'default');

get_template_part('loop-page', 'single-column');

etc. etc.

With regards to why we need a proper loop, see #13279.

Aha! Thanks for reminding me of that.

comment:9 @westi5 years ago

@demetris

If we want to do this then we need to not call them loop-archive and loop-page as they will then clash

get_template_part('loop-archive', 'author');
get_template_part('loop', 'archive');

Could load the same file?

comment:10 in reply to: ↑ 8 @nacin5 years ago

Replying to demetris:

As I see it, a main/parent theme should never use both args of get_template_part to specify its own template parts.

First, there is no reason to: A theme can, equally well, specify any file it wants by using only the first argument.

Second, there is reason not to: By doing so, it denies child themes the option to do something more specific for a template. So, it cancels the granularity and flexibility that the second arg of get_template_part gives us.

That's not true. The second argument provides additional context. If you wanted to override the loop in Twenty Ten, then you can use loop.php. If you wanted to override the loop for a specific instance, such as category.php, then you can use loop-category.php. Indeed, the loops for attachments, pages, and posts are customized from the default loop, and thus we use loop-$context.php in the parent theme.

But the point is, each of these are used exactly once [1], so there's no broader picture of overriding the loop of 10 files (loop.php) versus one file (loop-$context). If you want to override this, then you can use loop-page.php in the child theme. Absolutely no flexibility is lost, yet semantics remain the same and are consistent. Personally, I find the setup proposed by koopersmith to be ingenius.

[1] As mentioned earlier, page.php and the page template use the same loop, but this can be overridden in code with is_page_template().

comment:11 @demetris5 years ago

@Peter

As far as I understand, clashing is not an issue here.

We are only talking about a tiny namespace —a few files confined within the boundaries of one theme’s directory— which is controlled absolutely by the author of the theme. We are not talking about compatible name schemes across themes, or between themes and plugins, etc. etc.

Or am I missing something?

@nacin

I think you are overlooking my main concern:

(a) Twenty Ten is a theme that serves as a model .

(b) get_template_part is a function that we like and promote because of its advantages over plain includes: First, awaraness of child theme files. Second granular control (thanks to its second argument).

If (a) and (b) are true, why do we want to use get_template_part in Twenty Ten in a way that cancels or does not show off its second advantage?

About is_page_template, here is what I think. get_template_part can be implemented in ways that make customization VERY easy. Say, a user of your theme wants to have a completely static main menu. Sure, you reply to them: Make a child theme, make a nav.php file in it, and put this in the file:

<div id="menu">
    <ul class="some classes">
        <li>Your first menu item</li>
        <li>Your second menu item</li>
    </ul>
</div>

Then another user wants the same but only for Pages. Very easy again. They do the same as the first user, only name the file nav-page.php.

Thanks to get_template_part all this is possible without even a single line of PHP code.

Why not offer such simplicity and plain friendliness when we can?

comment:12 @nacin5 years ago

(In [15787]) Use get_template_part() for single.php, attachment.php, page.php, and our page template in Twenty Ten. Introduces loop-page/single/attachment.php. props koopersmith, fixes #15078 for 3.0.

Note: See TracTickets for help on using tickets.