Opened 6 years ago
Closed 5 years ago
#45984 closed enhancement (fixed)
Twenty Nineteen: Improve code organisation in template-functions.php
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | 5.4 | Priority: | normal |
Severity: | normal | Version: | |
Component: | Bundled Theme | Keywords: | has-patch needs-refresh good-first-bug |
Focuses: | Cc: |
Description
Originally reported by @grapplerulrich in the Twenty Nineteen GitHub repo:
The header in the template-functions.php
file is described as:
Functions which enhance the theme by hooking into WordPress
According to that description, all of the functions within template-functions.php
should hook into either a filter or an action of WordPress.
This is not the case for the following functions:
twentynineteen_can_show_post_thumbnail()
twentynineteen_image_filters_enabled()
twentynineteen_post_thumbnail_sizes_attr()
twentynineteen_get_avatar_size()
twentynineteen_is_comment_by_post_author()
twentynineteen_get_discussion_data()
twentynineteen_hsl_hex()
These are helper functions that would fit best in another file like inc/helper-functions.php
.
Also, twentynineteen_add_dropdown_icons()
could be moved to icon-functions.php
to join twentynineteen_nav_menu_social_icons()
.
Original ticket here: https://github.com/WordPress/twentynineteen/issues/548
Attachments (2)
Change History (13)
#2
@
5 years ago
- Milestone changed from Awaiting Review to 5.4
- Owner set to SergeyBiryukov
- Status changed from new to reviewing
#3
@
5 years ago
- Keywords needs-refresh good-first-bug added; good-first-issue removed
- Milestone changed from 5.4 to Future Release
Latest patch fails on trunk
, so this needs a refreshed patch. With 5.4 Beta 1 approaching, this is being moved to Future Release
. If any maintainer or committer feels this can be worked in before 5.4 Beta 1 or can assume ownership during a specific cycle, feel free to update the milestone accordingly.
#4
@
5 years ago
- Milestone changed from Future Release to 5.4
Note that twentynineteen_post_thumbnail_sizes_attr()
from the original list is hooked to wp_get_attachment_image_attributes
, so can be left as is.
#6
follow-up:
↓ 10
@
5 years ago
@SergeyBiryukov — did 47214 not include the new inc/helper-functions.php
file? I'm seeing a fatal error on trunk when running Twenty Nineteen right now:
#7
@
5 years ago
- Resolution fixed deleted
- Status changed from closed to reopened
It does seem like 47214 missed the new file. 45984-helper-functions-file.patch takes care of this. I had to recreate the file, since I couldn't find the correct patch above. Would someone mind giving this a look to confirm?
This ticket was mentioned in Slack in #core-themes by kjell. View the logs.
5 years ago
#9
@
5 years ago
I tested TwentyNineteen on trunk and verified the fatal error.
I then applied 45984-helper-functions-file.patch
and the theme works as expected.
I have moved the common functions in inc/helper-functions.php.
Also, twentynineteen_add_dropdown_icons() is moved to icon-functions.php (containing the required code).