Make WordPress Core

Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#49406 closed enhancement (fixed)

Introduce register_theme_support method

Reported by: kadamwhite's profile kadamwhite Owned by: timothyblynjacobs's profile TimothyBlynJacobs
Milestone: 5.5 Priority: normal
Severity: normal Version: 5.0
Component: Themes Keywords: has-patch has-unit-tests has-dev-note
Focuses: rest-api Cc:

Description (last modified by kadamwhite)

In #49037 support was added to list support for core theme features in the /themes endpoint. This patch was fairly conservative and manually safelisted specific theme supports by manually enumerating their expected schema in the controller.

Custom theme support values introduced by non-core plugins are not yet supported due to the lack of a safe way to validate the shape and content of their associated values. Several major plugins like WooCommerce and jetpack do implement their own theme-support features. While we can theoretically write out the schema for all high-profile plugins manually, this approach is both inequitable (what is "high-profile?") and would be unsustainably difficult to maintain.

The way that we're able to surface metadata to the API is by explicitly registering it using register_post_meta (and related functions). In #49037 therefore, @spacedmonkey proposed introducing a new register_theme_support method which core & third-party themes can use to describe the details of their features. This method could be used by plugins to describe the theme features they introduce, and the values used in that registration could then be used to safely populate theme support details in this endpoint.

Discussed in Slack on 2020-02-11

Change History (14)

#1 @kadamwhite
4 years ago

  • Description modified (diff)

#2 @georgestephanis
4 years ago

So to confirm on this, functionality would be similar to scripts / styles -- in that just because the theme support is /registered/ it isn't flagged on. Registering is just describing it.

I'd also suggest that we make sure theme support can be added before it's registered -- so that a late register doesn't break legacy declarations of adding it.

#3 @dshanske
4 years ago

Suggest, to figure out how to describe custom theme support properties, we ask for someone to provide a list of the custom ones the plugins are using and what they represent.

At the least, Jetpack and Woocommerce as represented in the ticket. Also, what properties do the built in ones have?

#4 @TimothyBlynJacobs
4 years ago

  • Keywords needs-patch added
  • Milestone changed from Awaiting Review to 5.5
  • Version changed from trunk to 5.0

This ticket was mentioned in PR #327 on WordPress/wordpress-develop by TimothyBJacobs.

4 years ago

  • Keywords has-patch has-unit-tests added; needs-patch removed

#6 @TimothyBlynJacobs
4 years ago

I've created a PR for a register_theme_feature() function. I opted for register_theme_feature instead of register_theme_support() because I think it is more clear that you are registering the feature that you can then add support for with add_theme_support( $feature ).

It follows the pattern of register_meta.

I've switched over all the core theme features to use the new API. I was also able to allow post-formats to work with this mechanism.

Cc'ing @sagarprajapati and @williampatton as component maintainers. Also cc'ing @spacedmonkey.

Registering is just describing it.

Yep registration just describes the feature.

I'd also suggest that we make sure theme support can be added before it's registered -- so that a late register doesn't break legacy declarations of adding it.

Yep this works. We're not checking against this in add_theme_support or requiring that the theme feature be registered before being used in any way.

This ticket was mentioned in Slack in #core-editor by timothybjacobs. View the logs.

4 years ago

#8 @williampatton
4 years ago

I haven't tested the patch but I gave it a quick pass over in the PR and it looks good, nice suite of added tests :)

The shift of name for the function makes sense to me. You register the _feature_ which becomes available and then theme can declare it has support for it.

If this were a theme only feature I would mark this with accept however I guess I should leave that to someone from REST component to accept it.

Looks real good to me though :D

#9 @TimothyBlynJacobs
4 years ago

Thanks for reviewing @williampatton!

This ticket was mentioned in Slack in #core-restapi by timothybjacobs. View the logs.

4 years ago

#11 @TimothyBlynJacobs
4 years ago

  • Owner set to TimothyBlynJacobs
  • Resolution set to fixed
  • Status changed from new to closed

In 48171:

Themes: Introduce register_theme_feature API.

Currently themes can declare support for a given feature by using add_theme_support(). This commit adds a register_theme_feature() API that allows plugins and WordPress Core to declare a list of available features that themes can support.

The REST API uses this to expose a theme's supported features if the feature has been registered with "show_in_rest" set to true.

Props kadamwhite, spacedmonkey, williampatton, desrosj, TimothyBlynJacobs.
Fixes #49406.

#12 follow-up: @desrosj
4 years ago

  • Keywords needs-dev-note added

Adding needs-dev-note for tracking purposes.

#13 in reply to: ↑ 12 @justinahinon
4 years ago

Replying to desrosj:

Adding needs-dev-note for tracking purposes.

Looks like it already have one

#14 @justinahinon
4 years ago

  • Keywords has-dev-note added; needs-dev-note removed
Note: See TracTickets for help on using tickets.