WordPress.org

Make WordPress Core

Opened 4 years ago

Closed 4 years ago

#41168 closed enhancement (fixed)

Identify the active theme when editing a site's themes

Reported by: johnbillion Owned by: johnbillion
Milestone: 4.9 Priority: normal
Severity: normal Version: 3.0
Component: Networks and Sites Keywords: has-patch commit
Focuses: administration, multisite Cc:

Description

On the Network Admin -> Sites -> Edit -> Themes screen, the currently active theme for the site is not identified.

There should be some form of identification of the active theme.

Attachments (9)

e5dd8abec08ff82a087403f168726f061.png (43.5 KB) - added by palmiak 4 years ago.
Screenshot of functionality
#41168.diff (1.3 KB) - added by palmiak 4 years ago.
patch
#41168.1.diff (1.3 KB) - added by palmiak 4 years ago.
Patch fix
41168.b.diff (833 bytes) - added by euthelup 4 years ago.
Replace the "Disable" button with the "Active Theme" flag when the theme is active
active_theme.png (244.0 KB) - added by euthelup 4 years ago.
Screenshot for the patch above
41168.c.diff (1.0 KB) - added by euthelup 4 years ago.
Add an "Active Theme" label next to the theme name.
active_theme2.png (56.4 KB) - added by euthelup 4 years ago.
screenshot for 41168.c.diff
41168.extra.diff (1.4 KB) - added by euthelup 4 years ago.
Extra: If marking a child theme differently than a template is a preference I've made this patch to support it.
41168.diff (1.4 KB) - added by johnbillion 4 years ago.

Download all attachments as: .zip

Change History (23)

#1 @palmiak
4 years ago

Just to be sure - you mean for example - http://localhost/wpmu/wp-admin/network/site-themes.php?id=xx

Of course it won't work with globally enable themes.

I attaching patch and a screenshot.

@palmiak
4 years ago

Screenshot of functionality

@palmiak
4 years ago

patch

@palmiak
4 years ago

Patch fix

#2 @euthelup
4 years ago

Hi,

Very good proposal and I'm eager to give you a hand with it.

From a UX perspective, the "Disable" button should be visible on the active theme row?
I rather see the "Active Theme" label taking the place of the "Disable" button since atm you can disable it, but the theme remains active. Imagine the confusion when the subsite admin switches the theme and it vanishes.

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


4 years ago

@euthelup
4 years ago

Replace the "Disable" button with the "Active Theme" flag when the theme is active

@euthelup
4 years ago

Screenshot for the patch above

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


4 years ago

#5 @earnjam
4 years ago

The mock-up in active_theme.png would still allow disabling the theme using the checkbox, so it's inconsistent. You could possibly not show the checkbox for that row, but I think the label solution in e5dd8abec08ff82a087403f168726f061.png is the better option.

I would lean toward just adding a label to indicate the active theme, but keeping the existing disable functionality. You can already network-disable themes that are active on some sites, and there is no good way to prevent that without querying every site on the network. Not to mention if that ability was removed, it would break a common paradigm of preventing new activations of older/deprecated themes on more open-facing networks (think wordpress.com)

If we are worried about an individual site admin changing their theme after it has been disabled, then the better catch-all option would be to provide a warning when switching away from a disabled theme that you won't be able to switch back. That would cover it being disabled at the network level or per-site level.

#6 @flixos90
4 years ago

I agree with what @earnjam says.

The "Active Theme" indicator should not replace the "Disable" row action, and it also shouldn't be placed in the row actions area, since it isn't one. In addition to the above arguments, you can also disable a theme network-wide although it's enabled somewhere, so we shouldn't prevent something similar here.

For the visuals, my suggestion would be to be something similar like in the Network Users table, in the way the Super Admin user is highlighted. It's more prominent and doesn't look like it's a row action that isn't one. :)

#7 @euthelup
4 years ago

Yeah, I agree with you guys, that would be inconsistent. Now if I think about it, blocking the disable action would be a capability task, not a UI issue.

I followed @flixos90 suggestion and made a small patch inspired by the Network Users table.
The only difference is that on Network the username is a link, and the badge is bold, but I made the badge lighter in font-size since the theme name is bold.

I'm attaching a patch and a screenshot, but as a new contributor, please let me know if it is bothering when I add screenshots.

@euthelup
4 years ago

Add an "Active Theme" label next to the theme name.

@euthelup
4 years ago

screenshot for 41168.c.diff

@euthelup
4 years ago

Extra: If marking a child theme differently than a template is a preference I've made this patch to support it.

#8 @euthelup
4 years ago

  • Keywords has-patch added; needs-patch removed

#9 @flixos90
4 years ago

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

I like the way this looks. We should get some actual designer feedback though.

I'll mark this good-first-bug ticket as "claimed".

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


4 years ago

@johnbillion
4 years ago

#11 @johnbillion
4 years ago

  • Keywords commit added; good-first-bug removed
  • Owner changed from euthelup to johnbillion
  • Status changed from assigned to reviewing

#12 @johnbillion
4 years ago

41168.diff tweaks this slightly so the styling is inline with other list tables (everything's within the strong tag).

#13 @johnbillion
4 years ago

  • Milestone changed from Awaiting Review to 4.9

#14 @johnbillion
4 years ago

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

In 41214:

Networks and Sites: Display an indicator for the active theme or child theme when viewing a site's themes within Network Admin.

Props palmiak, euthelup

Fixes #41168

Note: See TracTickets for help on using tickets.