Markup in core is too hard to find.

Moving all the tpl.php files into a /templates/ directory within the module that provides them will mirror our best practices for theme development, and give theme developers a consistent place to find templates.

As we're converting all theme functions to template files, we're going to be getting a lot more templates, and they'll need to be better organized. This change will not only help organize our current files, but will help pave the way for twig.

CommentFileSizeAuthor
#29 core-move-template-files-in-templates-folder-1760530-29.patch954 byteseric_a
#23 core-move-template-files-in-templates-folder-1760530-23.patch14.28 KBfabianx
#22 move-template-files-in-templates-folder-1760530-24.patch9.51 KBjustafish
#22 interdiff.txt601 bytesjustafish
#17 move-template-files-in-templates-folder-1760530-17.patch9.06 KBrisse
#9 move-template-files-in-templates-folder-1760530-9.patch8.98 KBcrazyrohila
#5 move-template-files-into-template-dirs-1760530-5.patch39.18 KBalbert volkman
#5 move-template-files-into-template-dirs-1760530-5-test-do-not-test.patch26.49 KBalbert volkman
#3 move-template-files-into-template-dirs-1760530-3.patch118.39 KBjenlampton
#1 move-template-files-into-template-dirs-1760530-1.patch109.25 KBjenlampton

Comments

jenlampton’s picture

jenlampton
she / her
commented
Status: Active » Needs review
Issue tags: +Novice
StatusFileSize
new move-template-files-into-template-dirs-1760530-1.patch109.25 KB

Here's a go at rearranging the template files.

Status: Needs review » Needs work

The last submitted patch, move-template-files-into-template-dirs-1760530-1.patch, failed testing.

jenlampton’s picture

jenlampton
she / her
commented
Status: Needs work » Needs review
StatusFileSize
new move-template-files-into-template-dirs-1760530-3.patch118.39 KB

Okay, this time let's make the theme check for the templates in those new directories.
(also moved seven templates, and removed the 'path' definition in hook_theme for toolbar module)

Status: Needs review » Needs work

The last submitted patch, move-template-files-into-template-dirs-1760530-3.patch, failed testing.

albert volkman’s picture

albert volkman commented
Status: Needs work » Needs review
StatusFileSize
new move-template-files-into-template-dirs-1760530-5-test-do-not-test.patch26.49 KB
new move-template-files-into-template-dirs-1760530-5.patch39.18 KB

Re-roll with renames instead of delete/creates. Also included a test only patch for easier testing.

@jenlampton If you run 'git config --global diff.renames copies', future git diffs will create rename patches so that a moved file's history is preserved.

Status: Needs review » Needs work

The last submitted patch, move-template-files-into-template-dirs-1760530-5.patch, failed testing.

crazyrohila’s picture

crazyrohila commented
Assigned: Unassigned » crazyrohila

assigning.

sun’s picture

sun
Primary language German
Location Karlsruhe
commented

Yeah, please use git's renames = copies config option to roll this patch. Significantly reduces the chance for having to re-roll the patch, too, because the actual file contents are not contained in the patch.

See our Configuring Git documentation. It's highly recommended to apply the entirety of recommended git options stated there.

The latest patch here contains many irrelevant changes for other issues though.

crazyrohila’s picture

crazyrohila commented
Status: Needs work » Needs review
StatusFileSize
new move-template-files-in-templates-folder-1760530-9.patch8.98 KB

Created patch by using renames/copies instead of create/delete.

Status: Needs review » Needs work
Issue tags: -Novice, -Twig, -theme system cleanup

The last submitted patch, move-template-files-in-templates-folder-1760530-9.patch, failed testing.

podarok’s picture

podarok
🇺🇦 he/him/his
Primary language Ukrainian
Location Ukraine
commented
Status: Needs work » Needs review

#9: move-template-files-in-templates-folder-1760530-9.patch queued for re-testing.

Status: Needs review » Needs work

The last submitted patch, move-template-files-in-templates-folder-1760530-9.patch, failed testing.

risse’s picture

risse commented
Status: Needs work » Needs review

#9: move-template-files-in-templates-folder-1760530-9.patch queued for re-testing.

Status: Needs review » Needs work
Issue tags: +Novice, +Twig, +theme system cleanup

The last submitted patch, move-template-files-in-templates-folder-1760530-9.patch, failed testing.

risse’s picture

risse commented

This patch is failing because it is still looking for system template files in core/modules/system, not core/modules/system/templates.

risse’s picture

risse commented
Status: Needs work » Needs review

Here is the revised patch, based on #9

risse’s picture

risse commented
StatusFileSize
new move-template-files-in-templates-folder-1760530-17.patch9.06 KB

Alright, new try.

Here is the revised patch, based on #9

podarok’s picture

podarok
🇺🇦 he/him/his
Primary language Ukrainian
Location Ukraine
commented

#17 looks good for me

bleen’s picture

bleen commented
Status: Needs review » Reviewed & tested by the community

I love this ...

Patch applies and everything on the theme layer seems to be working fine for me ... testbot seems happy. Combine that wil podarok's thumbs up in #18 and I'd say this is RTBC

catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Needs review

Why just modules and not themes?

podarok’s picture

podarok
🇺🇦 he/him/his
Primary language Ukrainian
Location Ukraine
commented

#20 agree with that!

justafish’s picture

justafish
she/her
Primary language English
Location London, UK
commented
Title: Move all template files provided by core modules into a 'templates' directory inside that module directory » Move all template files provided by core modules and themes into a 'templates' directory inside that module directory
Assigned: crazyrohila » Unassigned
StatusFileSize
new interdiff.txt601 bytes
new move-template-files-in-templates-folder-1760530-24.patch9.51 KB

The theme engine is already looking for template suggestions under all theme subfolders, so the seven templates just need moving.

fabianx’s picture

fabianx commented
Status: Needs review » Reviewed & tested by the community
StatusFileSize
new core-move-template-files-in-templates-folder-1760530-23.patch14.28 KB

That is good to get #1696786: Integrate Twig into core: Implementation issue more cleanly in.

RTBC and re-roll to apply cleanly to 8.x

fabianx’s picture

fabianx commented
Issue tags: +Needs change record

As this is important for contrib modules to know, I guess this needs a change notification.

catch’s picture

catch
he/him
Primary language English
commented
Title: Move all template files provided by core modules and themes into a 'templates' directory inside that module directory » Change notice: Move all template files provided by core modules and themes into a 'templates' directory
Priority: Normal » Critical
Status: Reviewed & tested by the community » Active

Committed/pushed to 8.x, thanks!

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented

I didn't see this issue until it showed up in the commit logs, but I must say, BRAVO!

eric_a’s picture

eric_a commented

This patch deals with the case where the 'path' property is not in use. If path is declared you cannot easily have include files in '[path]' and templates in '[path]/templates'. What would be the recommendation for modules that currently have templates and include files in a 'theme' directory?

eric_a’s picture

eric_a commented

Two things I've noticed so far for a follow-up.
The _theme_process_registry() doxygen is incorrect now.
The core/modules/field/theme/field.tpl.php template wasn't caught by the tests because it is not used and exists as a starting point for customization only.

eric_a’s picture

eric_a commented
Status: Active » Needs review
StatusFileSize
new core-move-template-files-in-templates-folder-1760530-29.patch954 bytes
tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
commented
Status: Needs review » Reviewed & tested by the community

Follow-up is RTBC.

chx’s picture

chx commented

I wrote the change notice so that once the followup is committed this can be set to fixed.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Title: Change notice: Move all template files provided by core modules and themes into a 'templates' directory » Move all template files provided by core modules and themes into a 'templates' directory
Priority: Critical » Normal
Status: Reviewed & tested by the community » Fixed
Issue tags: -Needs change record

Committed and pushed to 8.x. Thanks!

Sounds like this should be back to fixed now, given #31.

Automatically closed -- issue fixed for 2 weeks with no activity.