Opened 6 years ago
Closed 6 years ago
#46925 closed enhancement (fixed)
Site Health: Update 'Other Themes' to 'Inactive Themes' and give 'Parent Theme' it's own accordion.
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | 5.3 | Priority: | normal |
Severity: | normal | Version: | 5.2 |
Component: | Site Health | Keywords: | has-screenshots site-health has-patch commit |
Focuses: | ui | Cc: |
Description
Hello,
Currently, activating a child theme lists it on the 'Active Theme' accordion and leaves the parent theme in the 'Other Themes' list. I was originally seeing the 'Other Themes' list as the inactive list so could see it being confused.
It would be nice to list the parent either in it's own 'Parent Theme' section or along with the child making the accordion 'Active Themes' and updating the 'Other Themes' to be 'Inactive Themes' to match the status test for 'You should remove inactive themes'.
Thoughts?
Thanks
Attachments (9)
Change History (25)
@
6 years ago
Patch to introduce 'Parent Theme' accordion and rename 'Other Themes' to 'Inactive Themes'
@
6 years ago
Parent Theme section similar to Active Theme but without 'Parent theme' and 'Theme features' rows.
#1
@
6 years ago
- Keywords has-patch needs-testing has-screenshots added
I've uploaded 46925.diff to introduce a Parent Theme accordion section similar to Active Theme, and to rename the 'Other Themes' to 'Inactive Themes' to match the Info tests.
The Parent theme row isn't present in the Parent Theme section on purpose. As to the Theme Features as the Active Theme section retrieved that information from a global option it wasn't easy to do the same for the Parent Theme so omitted it as it's basically covered in the Active Theme anyway and would just be duplicate information.
#3
@
6 years ago
@garrett-eclipse tested above patch and found below errors.
Notice: Undefined variable: parent_theme_updates in /var/www/html/wordpress/wp-admin/includes/class-wp-debug-data.php on line 913
Warning: array_key_exists() expects parameter 2 to be array, null given in /var/www/html/wordpress/wp-admin/includes/class-wp-debug-data.php on line 913
@
6 years ago
Updated patch to address the warnings caused by use of non-existent $parent_theme_updates
#4
@
6 years ago
Thanks @mukesh27 I appreciate the catch there. Uploaded 46925.2.diff to address these issues. They're both simply related to my rename of the $theme_updates
accidentally to $parent_theme_updates
when I was updating the other variables I carried over for use int he parent theme accordion.
Can you give it a re-test and let me know if you have any further issues.
Cheers
#5
@
6 years ago
- Keywords has-patch removed
- Milestone changed from Awaiting Review to 5.3
The parent theme row under "Active theme" should probably show the slug as well, and the slug should also be shown in the parent theme section (this is one of the most common reasons why a parent theme isn't working I've noticed, they use the wrong slug, or try to use the name instead of the slug).
I see the parent theme is also missing its own slug, which would be good to include there somehow.
Beyond that, I think this is a good enhancement for 5.3
@
6 years ago
Refreshed patch to introduce slugs to active theme name, parent theme name and the parent theme found in the active theme section
#6
@
6 years ago
- Keywords has-patch added
Thanks @Clorith I appreciate the feedback and the milestoning.
I've updated the patch (46925.3.diff) to follow the convention from the other themes list to include the slug in parentheses which I've applied to the 'Name' field on both the active and parent sections as well as the 'Parent theme' field in the Active theme section.
Updated Debug Data;
### wp-active-theme ### name: Honeybadger ChildTheme (honeybadger-child-master) version: 1.0 author: XXXXX author_website: http:// www. xxx .ca parent_theme: Honeybadger (honeybadger-theme) theme_features: menus, post-thumbnails, automatic-feed-links, title-tag, html5, post-formats ### wp-parent-theme ### name: Honeybadger (honeybadger-theme) version: 0.1.10 author: XXXXX author_website: https:// www. xxx .ca
Good to go for testing here.
Cheers
#7
@
6 years ago
This is a nice addition thanks @garrett-eclipse !
46925.4.diff is based on 46925.3.diff .
Additions & changes:
1) Fixes some minor CS issues.
2) Changes
if ( $active_theme->stylesheet === $theme_slug || $parent_theme->stylesheet === $theme_slug ) {
to
if ( $active_theme->stylesheet === $theme_slug || ! empty( $parent_theme->stylesheet ) && $parent_theme->stylesheet === $theme_slug ) {
To avoid the Trying to get property 'stylesheet' of non-object
error if a parent theme is selected.
3) Converts the Theme directory location of the Active Theme to use get_stylesheet_directory()
instead.
4) Adds the Theme directory location to the Parent Theme as well with get_template_directory()
#8
@
6 years ago
Thanks @xkon I always appreciate the CS fixes. And for catching the non-object check, as well as the theme_path updates. This looks pretty good to go, maybe some more testing and we'll see if it needs a refresh when 5.3 rolls around.
P.S. I did make one update to the $all_themes exclusion. I was updating the preceding comment and felt it starting to cluster so I've separated the two exclusion check giving them each their own comment. 46925.5.diff
#9
@
6 years ago
@Clorith could you check the latest patch here in case we missed something so we can get this in maybe?
#10
@
6 years ago
I just tested this as part of my first ever core contribution, and it seems like the patch does what it needs to be doing.
It would be nice if add_theme_support actually used backtrace or something of the sort, to know which theme (child or parent) adds a specific feature support. But that's not related to this ticket.
#11
@
6 years ago
- Component changed from Administration to Site Health
Moving Site Health tickets into their lovely new home, the Site Health component.
#12
@
6 years ago
Thanks @msaggiorato for testing and I agree that backtrace would be nice, do you mind opening another ticket here on Trac for that suggestion. Cheers
#13
@
6 years ago
- Keywords commit added; needs-testing removed
46925.6.diff looks good, I like the approach, and the re-labeling of Other themes
to Inactive themes
makes much more sense from a reading perspective.
#15
@
6 years ago
- Summary changed from Site Health: Should the parent theme be listed in 'Active Theme' on info tab? to Site Health: Update 'Other Themes' to 'Inactive Themes' and give 'Parent Theme' it's own accordion.
*Updated title to provide a clear description of this tickets current direction for easier review
Currently, Parent theme is showing in 'Other Themes' section