Make WordPress Core

Opened 5 years ago

Closed 5 years ago

Last modified 4 years ago

#48798 closed enhancement (duplicate)

REST API: Expose editor-color-palette theme support in /themes endpoint

Reported by: koke's profile koke Owned by:
Milestone: Priority: normal
Severity: normal Version: 5.0
Component: REST API Keywords: has-patch
Focuses: rest-api Cc:

Description

In #45016, a new /themes endpoint was created to expose some of the supported theme features.

As part of the work in bringing in more features to gutenberg-mobile we'll need to access the color palette defined by themes. We are running across this in the Button block but I imagine it will start coming up in more places.

As a secondary issue, we currently don't have any supported method for authentication in the apps (waiting for basic auth in #42790 or the efforts in WP-API/authentication to materialize). I'm wondering if it would be OK to expose those settings to unauthenticated requests, since it doesn't seem like sensitive information, and it's likely publicly available somewhere in the theme's CSS.

Attachments (3)

48799-1.diff (4.8 KB) - added by koke 5 years ago.
Adds editor-color-palette to themes endpoint, with unit tests
48799-2.diff (10.3 KB) - added by koke 5 years ago.
Exposes editor-color-palette, editor-font-sizes, disable-custom-colors, disable-custom-font-sizes
48799-3.diff (10.6 KB) - added by koke 5 years ago.
v3 Schema changes

Download all attachments as: .zip

Change History (21)

@koke
5 years ago

Adds editor-color-palette to themes endpoint, with unit tests

#1 @koke
5 years ago

  • Keywords has-patch added

#2 @spacedmonkey
5 years ago

  • Version set to 5.0

Thanks for your ticket @koke

Can I recommend also adding some extra fields as well.

editor-font-sizes
disable-custom-colors
disable-custom-font-sizes

All of which are used by the editor.

I would also look at editor-gradient-presets and disable-custom-gradients

#3 @koke
5 years ago

@spacedmonkey good call, we'll certainly need the gradients soon enough. I don't think we'll use any custom fonts on mobile for now, but it seems like a good idea to include those.

I'm wondering if it would be better to keep the patch small and add those later, or if it's best to add them all at once.

#4 @SergeyBiryukov
5 years ago

  • Milestone changed from Awaiting Review to 5.4

#5 @spacedmonkey
5 years ago

I think that editor-gradient-presets and disable-custom-gradients should wait until these are in core. They are still in the gutenberg plugin and being tested.

As for the other fields, I don't see why not to add them here. Save code review later.

#6 @koke
5 years ago

I've sent a PR with an override on Gutenberg for the editor-gradient-presets, I can't find any references to disable-custom-gradients. https://github.com/WordPress/gutenberg/pull/18822

#7 @spacedmonkey
5 years ago

disable-custom-gradients can be found here

@koke
5 years ago

Exposes editor-color-palette, editor-font-sizes, disable-custom-colors, disable-custom-font-sizes

#8 @koke
5 years ago

Added an updated patch, here's the API output for quick reference:

$ http -ba admin:password http://localhost:8889/wp-json/wp/v2/themes status==active
[
    {
        "theme_supports": {
            "disable-custom-colors": false,
            "disable-custom-font-sizes": false,
            "editor-color-palette": [
                {
                    "color": "#cd2653",
                    "name": "Accent Color",
                    "slug": "accent"
                },
                {
                    "color": "#000000",
                    "name": "Primary",
                    "slug": "primary"
                },
                {
                    "color": "#6d6d6d",
                    "name": "Secondary",
                    "slug": "secondary"
                },
                {
                    "color": "#dcd7ca",
                    "name": "Subtle Background",
                    "slug": "subtle-background"
                },
                {
                    "color": "#f5efe0",
                    "name": "Background Color",
                    "slug": "background"
                }
            ],
            "editor-font-sizes": [
                {
                    "name": "Small",
                    "shortName": "S",
                    "size": 18,
                    "slug": "small"
                },
                {
                    "name": "Regular",
                    "shortName": "M",
                    "size": 21,
                    "slug": "normal"
                },
                {
                    "name": "Large",
                    "shortName": "L",
                    "size": 26.25,
                    "slug": "large"
                },
                {
                    "name": "Larger",
                    "shortName": "XL",
                    "size": 32,
                    "slug": "larger"
                }
            ],
            "formats": [
                "standard"
            ],
            "post-thumbnails": true,
            "responsive-embeds": false
        }
    }
]

It looks like twenty nineteen adds a shortName field when registering the font sizes, which is not part of the schema. I'm not sure if I should filter the output to only those properties defined in the schema or it's fine to leave that in the response.

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


5 years ago

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


5 years ago

#11 @TimothyBlynJacobs
5 years ago

@koke this is looking good to me. One thing is that when listing properties, you can't use a type shorthand like that. You need to specify a full schema.

'name'  => 'string',

needs to be

'name' => array(
    'type' => 'string',
),

It also looks like a short array syntax array snuck in there for editor-color-palette.

We also don't support the JSON-Schema v4+ required syntax, instead we use the v3 boolean syntax at the moment.

'required' => array( 'name', 'slug', 'color' ),

That being said, since this is a readonly property, we could just drop the required information entirely.

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


5 years ago

This ticket was mentioned in PR #138 on WordPress/wordpress-develop by koke.


5 years ago
#13

@koke
5 years ago

v3 Schema changes

#14 @koke
5 years ago

We also don't support the JSON-Schema v4+ required syntax, instead we use the v3 boolean syntax at the moment

@TimothyBlynJacobs I uploaded a new patch with those changes, but I'm a bit confused now because the schema definition says it's using v4:

'$schema'    => 'http://json-schema.org/draft-04/schema#'

#15 follow-up: @spacedmonkey
5 years ago

I think we should close this in favor of #49037 .

#16 in reply to: ↑ 15 @koke
5 years ago

Replying to spacedmonkey:

I think we should close this in favor of #49037 .

That one looks more complete so it sounds good to me. My only concern is that we are hoping to get this into the next release so the apps can start using custom colors soon. If the patch in #49037 seems ready to go for 5.4, then I'm happy.

#17 @TimothyBlynJacobs
5 years ago

  • Milestone 5.4 deleted
  • Resolution set to duplicate
  • Status changed from new to closed

I'm a bit confused now because the schema definition says it's using v4:

@koke yep, it's confusing! We essentially support v4 except for the required syntax. See also #48818.

I think we should close this in favor of #49037 .

I agree.

If the patch in #49037 seems ready to go for 5.4, then I'm happy.

I think we'll be able to get that patch in for 5.4.

Note: See TracTickets for help on using tickets.