Opened 10 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: |
|
Owned by: |
|
---|---|---|---|
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)
Change History (47)
#4
@
10 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
@
10 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
@
10 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
@
10 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.
This ticket was mentioned in Slack in #core-multisite by jeremyfelt. View the logs.
10 years ago
#10
@
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
@
8 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
@
8 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
@
8 years ago
@bradt I'm wondering if the school crossing translates across cultures? It may, just something I wondered.
#14
@
8 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, the user 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.
#16
@
8 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?
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.
#18
@
8 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
@
8 years ago
@Mista-Flo can you upload a screenshot of your patch please? It makes it easier for people to comment and see.
#20
@
8 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:
↓ 22
@
8 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
@
8 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 ?
#24
@
8 years ago
@karmatosed an anchor or a real link ? Because I don't know which URL you have in mind.
#25
@
8 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.
#26
@
7 years ago
@seanchayes I assume that yellow underline is to show the link not in the patch? Its a little misleading.
#27
@
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.
This ticket was mentioned in Slack in #design by karmatosed. View the logs.
7 years ago
#30
@
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
@
3 years ago
- Milestone changed from Future Release to 5.8
- Owner changed from dpik to SergeyBiryukov
- Status changed from assigned to reviewing
#33
@
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.
#34
@
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!
Makes sense. In the Appearance screen, this message is displayed once you click into a theme's details: