Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for nested attributes and post meta #5191

Closed
diegoliv opened this issue Feb 22, 2018 · 5 comments
Closed

Support for nested attributes and post meta #5191

diegoliv opened this issue Feb 22, 2018 · 5 comments
Labels
Core REST API Task Task for Core REST API efforts [Type] WP Core Ticket Requires an upstream change from WordPress. Core Trac ticket should be linked.

Comments

@diegoliv
Copy link

Issue Overview

Currently, Gutenberg handles variables on the attributes object. It works pretty well if you have simple attributes, like this:

attributes: {
    fontSize: {
        type: 'number',
	default: 10,
    },
},

Also, you can save this attribute as post meta by doing this:

attributes: {
    fontSize: {
        type: 'number',
	default: 10,
        source: 'meta',
        meta: 'font_size',
    },
},

And then, you declare the existence of this meta like this:

function my_blocks_meta_init(){
    register_meta('post', 'navigation_styles', array(
        'show_in_rest' => true,
        'type' => 'integer',
    ));
}
add_action('init', 'my_blocks_meta_init');

Which also works pretty well. But on some cases, you might end up with a lot of attributes, and if you want to save them as post meta, it's not practical to save each single attribute into it's own post meta. On this case, it makes sense to group attributes into a single one. Something like this:

attributes: {
    navigationStyles: {
        type: 'object',
	default: {
	    fontSize: {
                type: 'number',
                default: 10,
            },
	    fontColor: {
                type: 'string',
                default: '#ccc',
            },
	},
        source: 'meta',
        meta: 'navigation_styles',
    },
},

But there are two problems right now that prevents nested attributes to work:

1: props.setAttributes doesn't allow updates on nested objects

Currently, you can't do something like this:

const changeArrowSize = ( value ) => {
    props.attributes.navigationStyles.fontSize = value;
    props.setAttributes( { navigationStyles: props.attributes.navigationStyles } );
};

The only way you can update a nested attribute is by updating the whole attribute. So my current workaround is like this:

const changeArrowSize = ( value ) => {
    const styles = { ...props.attributes.navigationStyles };
    styles.fontSize = value;

    props.setAttributes( { navigationStyles: styles } );
};

Which is not practical, but works right now. I'm basically cloning the original attribute, changing the value, and updating the whole attribute.

2: register_meta type

I tried to use register_meta to set the navigationStyles attribute as the navigation_styles post meta, but it didn't worked. I suspect that it is related to the type parameter on the register_meta function:

register_meta( 'post', 'navigation_styles', array(
    'show_in_rest' => true,
    'type' => '??????', // what meta type should we use?
) );

The navigationStyles attribute is an Object on javascript, but we should save its post meta as a multidimensional array. I'm not sure if currently there's a way to set type as object or array. So, if we do have something that works with a nested attribute, please let me know! 😃

Expected Behavior

Nested attributes should be allowed by default. Also, props.setAttributes should allow updating a nested attribute. And the whole attribute (on my example, navigationStyles) should be saved as a single multidimensional array post meta.

Current Behavior

Attributes are only single level. To have a nested attribute it is necessary to hack your way around the way props.setAttributes works. Also, it's not clear what you should use for the type parameter for the register_meta function.

Possible Solution

Allowing a nested structure (something like my example above) would be great. Also, props.setAttributes could allow updating nested attributes like this:

const changeArrowSize = ( value ) => {
    props.setAttributes( { 'navigationStyles.fontSize': value } );
};
@youknowriad
Copy link
Contributor

AFAIK post_meta don't allow storing objects, a workaround for now is to use a unique attribute and use JSON encode it or something. I wonder if this issue should be raised in Trac first as Gutenberg can't support these types as meta if Core doesn't support it.

@diegoliv
Copy link
Author

@youknowriad yeah, you're right. based on your suggestion, I was able to hack my way around this by creating another attribute and storing the nested object as a string. Something like this:

const changeFontSize = ( value ) => {
	const styles = { ...props.attributes.navigationStyles };
	styles.fontSize = value;

	props.setAttributes( {
		navigationStyles: styles,
	} );

	updateNavigationStyles();
};

const updateNavigationStyles = () => {
	const attrStringStr = JSON.stringify( props.attributes.navigationStyles );

	props.setAttributes( {
		navigationStylesString: attrStringStr,
	} );
};

Then I register the post meta like this:

function blocks_meta_init(){
	register_meta('post', 'navigation_styles', array(
		'show_in_rest' => true,
		'type' => 'string',
		'sanitize_callback' => 'blocks_sanitize_attr'
	));
}

add_action('init', 'blocks_blocks_meta_init');

function blocks_sanitize_attr( $meta_value, $meta_key, $meta_type ){
	return serialize( json_decode( $meta_value ) );
}

With this, I can get the object doing a unserialize:

$nav = get_post_meta( get_the_ID(), 'navigation_styles', true );
echo '<pre>'. print_r( unserialize( $nav ) , true ) . '</pre>';

This works, but is not the optimal solution. I would love to create a ticket on Trac about this, but I'm not sure what we need to do and where. Can you give me some directions on what should I put on the ticket, or where we could change to allow this kind of stuff? Sorry if it's too much to ask, I would love to contribute but never did a Trac ticket before.

@youknowriad
Copy link
Contributor

Can you give me some directions on what should I put on the ticket, or where we could change to allow this kind of stuff?

I think the issue should be something like: "support associative array type in register_meta". The underlying implementation could rely on json_encode, json_decode or any other way to serialize/unserialize associative arrays in the database.

@diegoliv
Copy link
Author

@youknowriad thanks for the help! I just opened a ticket on Trac for this. Here is the link for reference: https://core.trac.wordpress.org/ticket/43392

@mtias mtias added [Type] WP Core Ticket Requires an upstream change from WordPress. Core Trac ticket should be linked. Core REST API Task Task for Core REST API efforts and removed [Type] WP Core Ticket Requires an upstream change from WordPress. Core Trac ticket should be linked. labels Mar 6, 2018
@mtias
Copy link
Member

mtias commented Mar 6, 2018

Added labels to refer to later and closing as there is matching core track ticket.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Core REST API Task Task for Core REST API efforts [Type] WP Core Ticket Requires an upstream change from WordPress. Core Trac ticket should be linked.
Projects
None yet
Development

No branches or pull requests

3 participants