WordPress.org

Make WordPress Core

Opened 12 months ago

Last modified 7 months ago

#41146 accepted enhancement

Add filter for a site's class (WP_MS_Users_List_Table)

Reported by: kraftbj Owned by: kraftbj
Milestone: 5.0 Priority: normal
Severity: normal Version:
Component: Networks and Sites Keywords: has-patch
Focuses: administration Cc:

Description

In the WP_MS_Users_List_Table, each site that is listed is span'd with a class of site-# where # is the ID of the network for a site.

Adding a filter to this class would allow site administrators to add addition classes to this output which would easily allow for styling of site items based on the additional classes.

Attachments (3)

41146.diff (1.1 KB) - added by kraftbj 12 months ago.
Adds filter to the span class
41146.2.diff (1.2 KB) - added by kraftbj 12 months ago.
converted to array and other suggested edits
41146.3.diff (1.3 KB) - added by kraftbj 12 months ago.
updated docs and other suggested changes

Download all attachments as: .zip

Change History (10)

@kraftbj
12 months ago

Adds filter to the span class

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


12 months ago

#2 @desrosj
12 months ago

  • Keywords needs-patch added; has-patch removed

Thanks for the initial patch @kraftbj. This sounds like a good addition. Some thoughts:

  • I think that this filter should pass an array of classes instead of a string. This will help prevent situations where someone adds a class but forgets to add a space, causing classes to bleed together.
  • The filter should be on its own line to make it more readable.
  • An array_unique() call would also help prevent duplicate classes.
  • Now that there is a filter, whatever value is output into the class attribute will need to be passed through esc_attr().

@kraftbj
12 months ago

converted to array and other suggested edits

#3 @kraftbj
12 months ago

  • Keywords has-patch added; needs-patch removed
  • Owner set to kraftbj
  • Status changed from new to accepted

Made the suggested changes. I'm only checking for is_array to make sure the return is in the right format. Someone can return false, etc to short-circuit completely.

We could also check for an empty array, but I don't want to overengineer it.

#4 @flixos90
12 months ago

@kraftbj

I'm only checking for is_array to make sure the return is in the right format.

I think we can even strip this out, as it's commonly expected to return the same type that is passed to a filter as the filtered parameter. Similar filters in core do not check this either.

We could also check for an empty array, but I don't want to overengineer it.

Right, I don't think that's necessary, especially since the default isn't empty either.

There are just a few (mostly naming) changes I'd like to suggest:

  • The variable could be called $site_classes to indicate that we're dealing with multiple classes (also makes it more obvious that it's an array).
  • While the site ID property is called $userblog_id (for legacy reasons), let's use a more appropriate name in the filter documentation (for example $site_id). The name is independent of the actual passed parameter anyway since it's a property.
  • It might be useful to pass the network ID (as saved in the $site_id property) to the filter as well, since that is also used in the default class. Similar to the above, the docs should probably call it $network_id.
  • Not very important, but I'd preferably use implode() instead of join() just because it is the actual and more commonly used PHP function, and since it aligns better with its counterpart explode().
  • Last but not least, let's look at the name of the filter ms_user_list_site_class for a bit longer. I'm not sure whether it's appropriate/precise enough. I'm not opposed to the current name, just something to think about further.

#5 @kraftbj
12 months ago

Cool, I'll make those changes.

I went with join vs implode after copying the body_class function. I couldn't recall/find if there was a preference in Core. I'll switch it over since, in the end, I want the committer happy with whatever they're committing ;-)

For the length of the filter name, I'm apathetic. Tried to mimic the naming used elsewhere ms_user_list_site_actions (noting how bad of an idea to follow MS naming schemes), but yeah, no preference on my end.

@kraftbj
12 months ago

updated docs and other suggested changes

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


11 months ago

#7 @kraftbj
7 months ago

  • Milestone changed from Awaiting Review to 5.0

Moving this into 5.0 to ensure consideration.

Note: See TracTickets for help on using tickets.