Skip to content
This repository has been archived by the owner. It is now read-only.

Add customize sections REST endpoint. #10

Closed
wants to merge 4 commits into from

Conversation

@miina
Copy link
Collaborator

@miina miina commented Jun 8, 2017

No description provided.

/** This action is documented in wp-includes/class-wp-customize-manager.php */
do_action( 'customize_register', $wp_customize );
}
$wp_customize->prepare_controls();

This comment has been minimized.

@miina

miina Jun 8, 2017
Author Collaborator

@westonruter Added the prepare_controls here since this will "fill in" the controls of a section. Do you think it's better to create a separate logic to detect the related controls instead? Seems that some get_item() tests with custom sections are failing when using prepare_controls. I haven't yet looked into that more thoroughly to detect the issue but wanted to ask your opinion about using prepare_controls like this here first.

This comment has been minimized.

@westonruter

westonruter Jun 8, 2017
Member

I think you're right to call it here. 👍

@miina
Copy link
Collaborator Author

@miina miina commented Jun 8, 2017

@westonruter Created the endpoint for sections as well. If you have time to take a look and let know if anything looks incorrect/missing, would be great.

*/
public function get_item_schema() {

$schema = array(

This comment has been minimized.

@westonruter

westonruter Jun 8, 2017
Member

Since the json methods in WP_Customize_Setion subclass can add additional params, it would probably be good to explicitly add the 'additionalProperties' => true, to the schema. That would mean that the additional props could be included even if the WP_Customize_Section didn't register the field's schema via register_rest_field().

This comment has been minimized.

@miina

miina Jun 9, 2017
Author Collaborator

@westonruter As far as I can read out from the schema documentation it's true by default. Do you think it's still better to add it for clarity? Will do that.

This comment has been minimized.

@westonruter

westonruter Jun 9, 2017
Member

OK, I wasn't aware it was true by default. Either way then.

@miina miina changed the title [WIP] Add customize sections endpoint. Add customize sections REST endpoint. Jun 22, 2017
@miina
Copy link
Collaborator Author

@miina miina commented Jun 22, 2017

@westonruter And same story here -- if you have time to check if anything seems to be incorrect or missing from the PR would be great.

A sample output:

    {
        "instance_number": 5,
        "id": "background_image",
        "priority": 80,
        "panel": "",
        "theme_supports": "custom-background",
        "title": "Background Image",
        "description": "",
        "controls": [
            "background_image",
            "background_preset",
            "background_position",
            "background_size",
            "background_repeat",
            "background_attachment"
        ],
        "type": "default",
        "description_hidden": false,
        "_links": {
            "self": [
                {
                    "href": "http://src.wordpress-develop.dev/wp-json/customize/v1/sections/background_image"
                }
            ],
            "controls": [
                {
                    "href": "http://src.wordpress-develop.dev/wp-json/customize/v1/controls/background_image"
                },
                {
                    "href": "http://src.wordpress-develop.dev/wp-json/customize/v1/controls/background_preset"
                },
                {
                    "href": "http://src.wordpress-develop.dev/wp-json/customize/v1/controls/background_position"
                },
                {
                    "href": "http://src.wordpress-develop.dev/wp-json/customize/v1/controls/background_size"
                },
                {
                    "href": "http://src.wordpress-develop.dev/wp-json/customize/v1/controls/background_repeat"
                },
                {
                    "href": "http://src.wordpress-develop.dev/wp-json/customize/v1/controls/background_attachment"
                }
            ]
        }
    }
@westonruter
Copy link
Member

@westonruter westonruter commented Jun 25, 2017

@miina as noted in another PR, I think instance_number should be removed.

Also, as I just noted elsewhere, the controls links should be using a standard child link relation.

I wonder if the panel prop should be expressed in the REST API as null instead of "" when it is not set. When it is set, then there should be an up link relation that points to the panel resource, as opposed to a custom panel relation.

@miina
Copy link
Collaborator Author

@miina miina commented Jun 29, 2017

@westonruter In this case should the theme_supports be null as well in case it's not set? The default values for both are "", do you think it's generally better practice to express values as null in REST API if they're not set (as opposed to the default "")?

Or you're thinking that in case of objects it should be null? If I'm not mistaken then here the panel is just panel's ID in WP_Customize_Section.

miina added 2 commits Jun 29, 2017
@westonruter
Copy link
Member

@westonruter westonruter commented Jul 3, 2017

@miina I think that in the REST API that an empty value like theme_supports or panel should be null as opposed to an empty string.

For example, when a post has a placeholder empty datetime as 0000-00-00 00:00:00 then it will get sent back as null instead of that underlying string: https://github.com/WordPress/wordpress-develop/blob/05325257c960909566a98a3f6d01ad082e0cfa1a/src/wp-includes/rest-api/endpoints/class-wp-rest-posts-controller.php#L916-L924

The comment resource's author_email property has a default of null as opposed to an empty string, though from looking at the source I think it will actually return an empty string regardless.

Nevertheless, the parent property of a post is expressed as 0 as opposed to null: https://github.com/WordPress/wordpress-develop/blob/05325257c960909566a98a3f6d01ad082e0cfa1a/src/wp-includes/rest-api/endpoints/class-wp-rest-posts-controller.php#L1508-L1509

So I think it's ultimately your call. In practice I think it won't matter to much sine they are both falsey values. Both should be accepted as input, but as output personally I think null is a cleaner value to represent an empty value. You may want to get a second opinion in #core-restapi.

@miina
Copy link
Collaborator Author

@miina miina commented Jul 6, 2017

Closing in favor of #15.

@miina miina closed this Jul 6, 2017
@miina miina deleted the customize-sections-rest-endpoint branch Aug 24, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.