Make WordPress Core

Opened 9 years ago

Closed 3 years ago

#30240 closed enhancement (fixed)

Tell if theme is a child theme in wp-admin/network/themes.php

Reported by: dpik's profile dpik Owned by: sergeybiryukov's profile SergeyBiryukov
Milestone: 5.8 Priority: normal
Severity: normal Version: 4.0
Component: Themes Keywords: good-first-bug has-ui-feedback has-patch
Focuses: administration, multisite Cc:

Description

When listing themes in wp-admin/network/themes.php (in network mode), i would find useful to see if a theme is a child theme.

Attachments (12)

Screenshot 2014-11-04 00.59.59.png (12.9 KB) - added by johnbillion 9 years ago.
30420-patched.png (12.0 KB) - added by dpik 9 years ago.
Patched version
30240.diff (744 bytes) - added by dpik 9 years ago.
Patch to implement #30240 feature request
Child-Theme.png (71.3 KB) - added by Travel_girl 8 years ago.
Child-Theme_2.png (79.6 KB) - added by Travel_girl 8 years ago.
30240.2.patch (1.3 KB) - added by Mista-Flo 7 years ago.
Update patch : Follow WP Coding standards
30240-no-theme-uri.jpg (78.1 KB) - added by seanchayes 7 years ago.
30240-with-theme-uri.jpg (76.4 KB) - added by seanchayes 7 years ago.
30240.3.patch (1.0 KB) - added by seanchayes 7 years ago.
child.png (21.8 KB) - added by Presskopp 6 years ago.
I'd like to see a link to the parent here (see screenshot)
30240.4.patch (995 bytes) - added by poena 3 years ago.
Refreshed the patch and removed aria-label.
ms-installed-themes.png (40.1 KB) - added by poena 3 years ago.
Current screenshot May 2021-30240.4

Download all attachments as: .zip

Change History (47)

#1 @johnbillion
9 years ago

  • Focuses template removed
  • Type changed from enhancement to feature request

Makes sense. In the Appearance screen, this message is displayed once you click into a theme's details:

https://core.trac.wordpress.org/raw-attachment/ticket/30240/Screenshot%202014-11-04%2000.59.59.png

#2 @johnbillion
9 years ago

  • Keywords needs-patch good-first-bug ui-feedback added

@dpik
9 years ago

Patched version

@dpik
9 years ago

Patch to implement #30240 feature request

#3 @dpik
9 years ago

  • Keywords has-patch added; needs-patch removed

#4 @jeremyfelt
9 years ago

  • Milestone changed from Awaiting Review to 4.1

Thanks for the patch @dpik, it works well and I think it's a great addition. I'll personally find this useful.

A couple tips:

  • If possible, create the patch against the root of the develop repository so that the patch can apply easily. This is for the convenience of any testers.
  • Per the WordPress PHP coding standards, braces are used on all blocks, even when not required. This is admittedly confusing when a file being patched already has many instances of the opposite. The others can stay as is, but the new lines should conform to the standards.

I'm going to let this sit for a bit to get some UI feedback.

#5 @bradt
9 years ago

<2¢>The theme title and description usually mentions that it is a child theme, so this seems a bit redundant to me and may add noise to the UI in the majority of cases.</2¢>

#6 @jacklenox
9 years ago

@bradt But this often isn't the case. I think the patch is good. It doesn't add any real clutter and it's useful to see this info without having to rely on theme developers adding it.

#7 @helen
9 years ago

  • Milestone changed from 4.1 to Future Release
  • Type changed from feature request to enhancement

I don't love the idea of putting this at the end of the meta items/links, especially thinking about line breaks on narrower screens. Don't have an alternate idea right now, but we're in beta for 4.1 now in any case, so punting.

#8 @DrewAPicture
9 years ago

  • Owner set to dpik
  • Status changed from new to assigned

This ticket was mentioned in Slack in #core-multisite by jeremyfelt. View the logs.


9 years ago

#10 @Travel_girl
8 years ago

I think it is a good idea to mark Child-Themes as Child-Themes, as you only can noticed that, if you label the Child-Theme as a Child, while creating it.

I agree with Helen, that it is not a good solution by adding the Child Information in the meta items, as the space is really limited there.

What about a Infobox above --> https://core.trac.wordpress.org/attachment/ticket/30240/Child-Theme.png

Or I don't know, if this is possible, but I also like this Version: --> https://core.trac.wordpress.org/attachment/ticket/30240/Child-Theme_2.png

What do you think?

#11 @karmatosed
7 years ago

@Travel_girl thanks for this idea. I'm not overly keen on having such a large visual cover on the screenshots. Those are small as they are. Is there any other way that doesn't involve such a prominent placing either in middle or over top?

#12 @bradt
7 years ago

Agreed. I think a small, semi transparent school crossing sign icon in the top right of the screenshot would be indication enough that it's a child theme. On hover it could become less transparent and the title attribute would say "Child Theme". This is what I mean by a school crossing sign: https://mistakenforarealpoet.files.wordpress.com/2011/11/my-word-school-safety.jpg.

#13 @karmatosed
7 years ago

@bradt I'm wondering if the school crossing translates across cultures? It may, just something I wondered.

#14 @bradt
7 years ago

@karmatosed Even if the sign isn't common in a given region, I think the silhouette of an adult escorting a child should still be decipherable. If there's another symbol that would more clearly portray "child", that would be better, but I can't think of one. Also, they can always click/tap on the theme for more details and we could expand upon that symbol in the details, showing "Child Theme" next to it.

Version 0, edited 7 years ago by bradt (next)

#15 @Presskopp
7 years ago

You may want to take a look how this is done in the following plugin:

https://wordpress.org/plugins/childify-me/screenshots/

#16 @karmatosed
7 years ago

@bradt I understand that point, however I do really feel that a symbol isn't the right route this time. As an aside, I do worry about the emotive aspect of a child symbol. Therefore, just the word to me makes sense or some way to denote that doesn't include iconography.

I almost feel we need to strip this back. If we can avoid interference with the screenshot then we can also make a patch easier. What about this?

https://cldup.com/tbHnf3Gkri.png

This has a few things that I think are of benefit:

  • The 'Child' link goes to parent theme. I'm not sure we need to say it's name as much as reference and indicate state.
  • There should be less translation and other issues this way.
  • It's pretty low impact and therefore benefits without having to do anything really new.
  • Still on scanning you can see the child themes.

#17 @bradt
7 years ago

👍

@Mista-Flo
7 years ago

Update patch : Follow WP Coding standards

#18 @Mista-Flo
7 years ago

I have updated the patch to follow WordPress coding standards.

By the way, I did not understood why the subject of this ticket have diverged on single site UI, it was basicly only for network theme list.

#19 @karmatosed
7 years ago

@Mista-Flo can you upload a screenshot of your patch please? It makes it easier for people to comment and see.

#20 @Mista-Flo
7 years ago

@karmatosed Hello, this is the same screenshot of the first patch https://core.trac.wordpress.org/attachment/ticket/30240/30420-patched.png

My updated patch is just to respect WordPress coding standards if the patch is accepted.

#21 follow-up: @karmatosed
7 years ago

Is the child theme name a link? I'm thinking it may make sense.

I also don't see an issue if we do work on both network and none - there was interest on both, however we can focus back onto network to get something in.

#22 in reply to: ↑ 21 @Mista-Flo
7 years ago

Replying to karmatosed:

Is the child theme name a link? I'm thinking it may make sense.

I also don't see an issue if we do work on both network and none - there was interest on both, however we can focus back onto network to get something in.

No it's not. It's a strong tag. What kind of link do you imagine ?

#23 @karmatosed
7 years ago

Maybe a link to the parent? I sort of feel it could be useful.

#24 @Mista-Flo
7 years ago

@karmatosed an anchor or a real link ? Because I don't know which URL you have in mind.

#25 @seanchayes
7 years ago

I updated this patch adding in a link to the parent theme if a ThemeURI is found and if not, just a name - the two image attachments show each variant.

I've attached the updated patch - 30240.3.patch - too.

@seanchayes
7 years ago

#26 @karmatosed
7 years ago

@seanchayes I assume that yellow underline is to show the link not in the patch? Its a little misleading.

#27 @seanchayes
7 years ago

@karmatosed Yes - correct. It is to highlight the difference in the screenshot for this ticket and is not included in the patch.

#28 @Mista-Flo
7 years ago

  • Component changed from Networks and Sites to Themes

This ticket was mentioned in Slack in #design by karmatosed. View the logs.


7 years ago

@Presskopp
6 years ago

I'd like to see a link to the parent here (see screenshot)

#30 @joyously
6 years ago

  • Keywords has-ui-feedback added; ui-feedback removed

This ticket was originally for the Network Themes list, but I think it makes sense to be consistent and indicate that a theme is a child theme everywhere it is listed.

I suggest that the same method be used on the Network Themes list and on the normal Themes page, as shown in comment 16: The word "Child" is first, linked to the local copy of the parent theme, followed by the child theme name. And it also makes sense to make the Details view show a link to the parent theme, instead of just the name.

This ticket was mentioned in Slack in #core by sergey. View the logs.


3 years ago

#32 @SergeyBiryukov
3 years ago

  • Milestone changed from Future Release to 5.8
  • Owner changed from dpik to SergeyBiryukov
  • Status changed from assigned to reviewing

@poena
3 years ago

Refreshed the patch and removed aria-label.

#33 @poena
3 years ago

When I tested patch 30240.3, it did not apply to the correct line.
Patch 30240.4 updates the line numbers and removes the aria-label that only repeated the theme name.

@poena
3 years ago

Current screenshot May 2021-30240.4

#34 @SergeyBiryukov
3 years ago

Thanks for the patch!

Just noting that esc_attr() or esc_url() are not needed here, because WP_Theme::display() already applies the appropriate escaping via the WP_Theme::markup_header() method, and that is how these values are displayed elsewhere in the file. There is also no point in translating strings like __( '%s' ), it can just be printed directly.

Replying to joyously:

This ticket was originally for the Network Themes list, but I think it makes sense to be consistent and indicate that a theme is a child theme everywhere it is listed.

I suggest that the same method be used on the Network Themes list and on the normal Themes page, as shown in comment 16: The word "Child" is first, linked to the local copy of the parent theme, followed by the child theme name. And it also makes sense to make the Details view show a link to the parent theme, instead of just the name.

I agree it would be great to aim for a consistent indication, however I wonder if that is feasible with the several different parts of the themes UI that we have (outlined in comment:36:ticket:48491). Specifically, the theme details modal and the Network Themes list table look quite different and have different UI elements. I also wonder if linking the word "Child" to the parent theme is clear enough and not confusing in terms of accessibility.

So I would like to go back to the original focus of the ticket on Network Themes screen.

Looking at the current patch though, it includes a link to the parent theme's URI instead of a local link to the parent theme details modal. I wonder if the latter would be more helpful, however that is something the current implementation in that same modal does not have, as seen in Screenshot 2014-11-04 00.59.59.png.

So I would suggest going with a simpler patch here, along the lines of 30240.2.patch, which would look like 30420-patched.png. That seems consistent enough to me for now :)

If someone feels strongly about making the parent theme name a link to the theme's URI or theme details modal, or making any other changes to unify the parent theme indication in various places, please create a new ticket. Thanks!

#35 @SergeyBiryukov
3 years ago

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

In 50978:

Themes: Add an indication of whether a theme is a child theme on network admin Themes screen.

This shows the parent theme name in a child theme's metadata section in the list table, in a similar way it is displayed in the theme details modal on the single site Themes screen.

Props dpik, Mista-Flo, seanchayes, poena, johnbillion, jeremyfelt, bradt, jacklenox, helen, Travel_girl, karmatosed, Presskopp, joyously, SergeyBiryukov.
Fixes #30240.

Note: See TracTickets for help on using tickets.