Path not displayed for current theme

[scribu]

When on wp-admin/themes.php all the themes have a helpful paragraph that goes like this:

All of this theme’s files are located in /themes/some-theme.

Sadly, this bit of information is not displayed for the current theme.

[CoenJacobs]

Shows the path of current theme. Shows both parent and current paths when current theme is a child theme.

[CoenJacobs]

If unused vars could be removed, the code get's cleaner. Var title has a double anyway.

[CoenJacobs]

Seems to work perfectly after removing unused vars. Not sure if they should be saved for translations though. If so, first patch is the way to go.

[scribu]

You should make a private* helper function, to avoid all the duplication.

*i.e. name is prefixed with an underscore.

[CoenJacobs]

Moved duplicate code to private function.

[CoenJacobs]

Not quite sure about the way I pass the required theme vars in class-wp-themes-list-table.php (line 176), but this way I can pass a subset of the entire theme array, just what I need. Array keys are with capitals and spaces in that file, so we always need a way to convert them to lowercase and underscore versions (to match the object variabels from wp-admin/themes.php).

[scribu]

Looks pretty good, except for the brace style on the method:

​http://codex.wordpress.org/WordPress_Coding_Standards#Brace_Style

... and the missing space after the first if.

[CoenJacobs]

Fixed two Coding Standards issues

[CoenJacobs]

Got that fixed. Thank you scribu!

[nacin]

As the method is used outside the class, it should not he underscored. A doc block would also be nice.

Would like a screenshot and UX approval on this. Don't want someone to look at this as unnecessary clutter.

[CoenJacobs]

New patch with doc block and remove the underscore for function

[CoenJacobs]

Screenshot of new situation: Theme

[CoenJacobs]

Screenshot of new situation: Child Theme

[CoenJacobs]

Added new patch with doc block and removed the underscore for function.

Also added two screenshots with new situation of a theme and a child theme.

[CoenJacobs]

Can I get some ui en ux feedback in here? Would be too bad if this doesn't make it through feature freeze, just because it is lost somewhere in the archives. ;)

[jane]

Can't a child theme add additional templates? That makes the patch text not accurate.

[CoenJacobs]

That goes the same for the text that is there right now for the available themes.

[scribu]

Yeah, if we're going to do this, we should change the wording everywhere. One suggestion:

Parent theme files are located in twentyeleven. Child theme files are located in twentyeleven-child.

[CoenJacobs]

I'm with scribu here. Changing the lines to a way that is correct in all forms of parent/child theme combinations. I can change my patch to suit the new text.

[scribu]

Please do. (Sidenote: no guarantee it will get into 3.3, but at least it will be ready for 3.4).

[CoenJacobs]

New patch, new text, new screenshots. All ready for 3.3. ;)

Text for non-child theme stays the same: "All of this theme’s files are located in /themes/twentyten." seems to be pretty good.

Edit: Something clearly went wrong with my previous uploads. Did my trick again and now they're all good.

[CoenJacobs]

Textual changes based on previous patch.

[CoenJacobs]

New text display for inactive child theme.

[CoenJacobs]

New text display for active child theme.

[scribu]

How about instead of:

Parent theme files are located in /themes/twentyeleven. Child theme files are located in /themes/twentyeleven-child. TwentyEleven Child uses templates from Twenty Eleven.

we shorten it to a single sentence:

TwentyEleven Child (/themes/twentyeleven-child) uses templates from Twenty Eleven (/themes/twentyeleven).

[CoenJacobs]

New diff, combining all vars to a new single line of text.

[CoenJacobs]

New text display for active child theme (with patch 2).

[CoenJacobs]

New text display for inactive child theme (with patch 2).

[CoenJacobs]

Some patches got merged on my end, so previous patches had some other changes in them as well. My bad, since patch 17944-textual-2.diff, there are no more problems with that.

Replying to scribu:

TwentyEleven Child (/themes/twentyeleven-child) uses templates from Twenty Eleven (/themes/twentyeleven).

That's a lot better indeed. Worked it out in a new patch + screenshots.

[scribu]

Looks pretty good.

[JohnONolan]

The alignment on 17944-new-text-active-2.png is slightly strange - I would keep the margin-left created by the theme screenshot and have the text align with the other text above it. Full screenshot rather than crop would be helpful - to get a better idea of information hierarchy.

[CoenJacobs]

Fullscreen screenshot

[CoenJacobs]

Fullscreen screenshot with Twenty Ten active

[CoenJacobs]

Replying to JohnONolan:

The alignment on 17944-new-text-active-2.png is slightly strange - I would keep the margin-left created by the theme screenshot and have the text align with the other text above it. Full screenshot rather than crop would be helpful - to get a better idea of information hierarchy.

Added two fullscreen screenshots. I think the alignment is correct this way. To illustrate that, I've added a screenshot of an active Twenty Ten install, where the new paragraph is aligned next to the screenshot. Here you can see that the margin is the same on both sides.

[JohnONolan]

Yep - you're right, I take it back. Looks pretty good to me, only other thing that stands out to me personally is that I would probably opt to say "uses template files from /xxx/" rather than "uses templates from /xxx/" - but Jane will be able to offer better insight on that one as she has a far wider knowledge of the nuances between terms like this throughout the rest of core.

[CoenJacobs]

With all the new improvements for themes, this looks like a nice little enhancement for 3.4.

Patch is all done. Only need UI/UX feedback and the text might need a little look at.

[CoenJacobs]

Right, this is 3.5-early period, so I'm not gonna let this one slip another version. :)

We might need to reconsider the text, but UI/UX feedback shouldn't be too hard to get, right?

[nacin]

We now hide tags, and bury file locations inside a 'Details' link by default.

I've always thought of these file paths as potentially useful but ultimately extraneous information for the vast majority of users, not to mention an eyesore. Do we honestly need them? I'm not decreeing "No," but I'm suggesting we need to really think about it.

As a practical matter, this needs to be updated to use WP_Theme.

Secondarily, there is the potential that a parent theme is invalid for the current theme on disk (and in WP_Theme) but not for what is the current 'template' option in the DB. This makes for fun failures: 20921#comment:1.

[CoenJacobs]

New situation with a standalone theme

[CoenJacobs]

New situation with a child theme

[CoenJacobs]

Thanks nacin, I have updated the patch to the way the themes screen now works. Also added two screenshots of a standalone theme and a child theme.

Does this patch need to check for the potential invalid theme, or is that something we need to catch elsewhere?

[CoenJacobs]

Updated patch to match new WordPress 3.4 themes screen

[lessbloat]

I'm new around these parts, sorry for just jumping in here near the end of the thread.

Two quick thoughts:

1) My gut says that 80% of WP users would not find this information useful. I have no way to quantify this guess, it's just a hunch. With that said, I'd almost prefer that we not add it.

2) If we do decide to add it, can we:

• Remove the "Twenty Eleven child uses templates from Twenty Eleven. Changes made to the templates will affect both themes." lines when a child theme is selected.
• Transition the "All of this theme's files are located in: themes/twentyeleven" line up to the end of the monster description paragraph above.

[ryan]

I too would rather remove the locations.

[lessbloat]

17944-remove.diff simply removes the locations.

[nacin]

I think this information is useful in one way only: It is the only way in the UI (outside of the theme editor) to identify that a theme is a child theme of another theme.

The question is, useful for whom? Is this useful for few, some, or most users?

If we determine it is useful information, we can simply include a line "Child theme of %s." That's, at most, what I'd like to put here.

[nacin]

In [21650]:

Remove paths to where theme files are located from theme details on themes.php. props lessbloat, see #17944.

[scribu]

+1 for "Child theme of %s". Users that have access to that screen need to understand that relationship, lest they go about deleting the parent theme.

[helenyhou]

Another +1 for "Child theme of %s"

[nacin]

In [21816]:

Indicate on themes.php when a theme is a child that requires a parent theme. fixes #17944.

[nacin]

In [21817]:

Undo nested paragraphs added in [21816]. see #17944.