WordPress.org

Make WordPress Core

Opened 4 years ago

Closed 2 years ago

Last modified 2 years ago

#11216 closed enhancement (fixed)

Support page templates located in a subdirectory of the theme

Reported by: westi Owned by: nacin
Milestone: 3.4 Priority: normal
Severity: normal Version: 2.9
Component: Themes Keywords: has-patch needs-testing
Focuses: Cc:

Description

#10959 hi-lighted that we displayed these page templates but they didn't work.

For 2.9 we stopped displaying them.

In the future we should support them

Test cases in WordPress tests: test_admin_includes_theme.php::test_page_templates_subdir()

Attachments (3)

11216.diff (1.8 KB) - added by rzen 4 years ago.
11216.2.diff (3.2 KB) - added by rzen 4 years ago.
Extends nested page templates to child themes
11216.3.diff (4.0 KB) - added by rzen 2 years ago.
Creates helper function for recursively searching through entire theme directory

Download all attachments as: .zip

Change History (26)

comment:1 dphiffer4 years ago

  • Cc dphiffer added

comment:2 nathanrice4 years ago

I'll try to work up a patch for this. I don't think it wastes any execution overhead to include them, but just need to figure out a way to get the entire relative path included as the value, for instance, 'folder/file.php' or 'folder/folder/file.php' ...

comment:3 scribu4 years ago

  • Keywords needs-patch added

comment:4 rzen4 years ago

  • Cc rzen added
  • Version set to 3.1

I'm jumping on board with this patch. We'll see how it goes

rzen4 years ago

comment:5 rzen4 years ago

It looks like whatever was breaking the page template selector was resolved through another patch, because simply removing the lines from get_page_template() that prevent including subdirectories allowed everything to work as expected.

To accommodate the 'folder/folder/template.php' hierarchy that nathan mentions above (and that I would also like to use) I've modified /wp-includes/theme.php to loop through a second level. The variable names could probably be better. I also went ahead and added a filter of 'available_page_templates', at the suggestion of the ever-brilliant filosofo, to the output of get_page_template() should theme devs want to specify their own array of templates at an even greater depth.

Let the record show that this was my first attempt ever at patching core files. Review the patch at your own risk.

comment:6 rzen4 years ago

  • Keywords has-patch added; needs-patch removed

comment:7 scribu4 years ago

Does this work with child themes as well? That was the reason why we disabled it before.

comment:8 rzen4 years ago

Ah, nope. It works for templates nested in their parent themes, but not templates nested in child themes themselves. I'm uploading a new patch to extend the same directory depth for child themes as well.

rzen4 years ago

Extends nested page templates to child themes

comment:9 scribu4 years ago

  • Keywords needs-testing added
  • Type changed from feature request to enhancement

comment:10 scribu4 years ago

There's a lot of duplicated code there. Consider putting it into a helper function.

comment:11 scribu3 years ago

Related: #13265

rzen2 years ago

Creates helper function for recursively searching through entire theme directory

comment:12 rzen2 years ago

  • Version changed from 3.1 to 3.3

After 14 months, I finally worked up the courage to take scribu's advice and create a helper function. This function eliminates all the redundant code I introduced in 11216.2.diff as well as the repeated code from the original loops for collecting the template files.

With this patch, WordPress can support page templates in any level of subdirectory within a theme, both for parent and child themes, and not lose track of it's location.

I've personally tested it with page templates nested as deep as 2 directories for both a parent and child theme.

comment:13 SergeyBiryukov2 years ago

  • Version changed from 3.3 to 3.1

Version field indicates when the enhancement was initially suggested.

comment:14 rzen2 years ago

  • Version changed from 3.1 to 2.9

Ah, got it, I wondered about the redundancy between version and milestone. In that case, it is actually from 2.9.

Technically it's a bugfix, but I won't touch that dial. ...though, I'd really love to see this slink in to 3.3 because of it :)

comment:15 nacin2 years ago

Let's remove the code from theme.php rather than commenting it out. Also, $ignore should be = array() and we should array_merge() in .svn, .git, ., and .. -- there's never a reason for those to be searched.

At the same time, this is actually doing three loops through the directories, rather than two. Perhaps there's a better way to clear up the abstraction.

Definitely not 3.3 material, but could go into 3.4.

comment:16 nacin2 years ago

  • Milestone changed from Future Release to 3.4

With WP_Theme, this is now an easy win.

comment:17 nacin2 years ago

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

In [20318]:

Support page templates located in a subdirectory of the theme. fixes #11216.

comment:18 follow-up: toscho2 years ago

  • Cc info@… added

There are some methods with @access public in PHPDoc while their visibility is set to private: _name_sort(), _name_sort_i18n() and scandir().

I think @access is obsolete when a visibility keyword is used in the method declaration.

comment:19 scribu2 years ago

Agreed. @access should only be used for private functions, since we can't use namespaces yet.

comment:20 in reply to: ↑ 18 nacin2 years ago

Replying to toscho:

There are some methods with @access public in PHPDoc while their visibility is set to private: _name_sort(), _name_sort_i18n() and scandir().

I think @access is obsolete when a visibility keyword is used in the method declaration.

I think I am going to make scandir() private, but _name_sort() and _name_sort_i18n() are public only because they are used as a callback. Their usage is private. Hence @access private.

You're probably right about @access not being necessary when a visibility keyword is in play. However, @access private specifically hides it from phpdocumentor and I am wondering if it overrides a visibility keyword.

Happy to remove @access private, and make scandir() private. New ticket?

comment:21 nacin2 years ago

Sorry, I got confused for a second there. _name_sort() and _name_sort_i18n() are indeed private, and can be since they are used by sort_by_name(). Their docs are wrong.

comment:22 nacin2 years ago

In [20324]:

Correct docs for WP_Theme scandir(), _name_sort(), and _name_sort_i18n() methods. All of them are private. see #11216, #20103.

comment:23 nacin2 years ago

In [20327]:

Set WP_Theme::scandir()'s default depth to 0 (flat), as intended. Cache untranslated page template names. Properly merge in a parent theme's templates when dealing with cached values. Always look one level deep for WP_Theme->get_files() regardless of file type. see #20103. see #11216.

Note: See TracTickets for help on using tickets.