Skip to content

Conversation

@Mte90
Copy link
Contributor

@Mte90 Mte90 commented Mar 13, 2019

Description

immagine
This pr enable with a new parameter in the metabox to change the cmb2 slug with something custom without creating conflicts with other mentabox in REST api.

Motivation and Context

For transaprence and improve the code organization this improve the quality of the schema of CMB fields.

Risk Level

This code shouldn't create issues with backward comaptibility because cmb2 is a default value.

Testing procedure

Create a new metabox, enable show_in_rest and also rest_group_values and define a slug.
Probably we need a better parameter that is more explicative then this one about the purpose.

Checklist:

public function __construct( CMB2 $cmb ) {
$this->cmb = $cmb;
self::$boxes[ $cmb->cmb_id ] = $this;
self::$rest_slug = 'cmb2';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Mte90 What's the point of this line (127) when we have a predefined value on line 81?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I saw that in case of multiple metabox where the parameter is not defined without that line, in other rest pages where should be the default value was replaced.
With forcing a reset the code was working and keeping backward compatibility.

@jtsternberg
Copy link
Member

I'm going to leave this PR open, as it's described a prototype, but I won't merge as-is. The reason is that I don't want someone using the rest_slug_group group to inadvertently change the namespace for everyone else using CMB2. Instead we will need to put only the fields within that specific box in that new namespace, so will require a bit of structure change (#1239 (comment)).

@Mte90
Copy link
Contributor Author

Mte90 commented Mar 14, 2019

Yes it is only a prototype that works and not create issues with other metabox as it is right now.
I prefered to do a pr just in case someone need an implementation or to have something to start discussing.
I can work on improve the code style anyway, it is not a big problem :-)

@jtsternberg
Copy link
Member

I appreciate the effort here. Will look into testing this as soon as I can.

@Mte90
Copy link
Contributor Author

Mte90 commented Mar 14, 2019

Well CMB2 is very important for my job since years so I am happy to contribute to it as much I can :-)

@Mte90
Copy link
Contributor Author

Mte90 commented Nov 11, 2019

any updates for this pr?

@jtsternberg
Copy link
Member

This PR does not work. It probably works in your tests because you only have 1 or 2 metaboxes and test types, and registered your rest_slug_group in the right order. it doesn't work because the register_rest_field methods happen in a static context, and do load like you think (per CMB2 object). In order to fix this, it will need some significant infrastructure changes.

@Mte90
Copy link
Contributor Author

Mte90 commented Nov 11, 2019

Quite sad but I hope for a fix that works in all the cases :-)

@jtsternberg
Copy link
Member

@Mte90 I just pushed a couple updates to the develop branch that will allow registering your own handlers for register_rest_field. This example replaces the rest slug, which you're trying to accomplish.
https://gist.github.com/jtsternberg/a70e845aca44356b8fbf05aafff4d0c8

It's a bit more verbose than what you were trying to do, but it does work, and provides ultimate flexibility.

@Mte90 Mte90 deleted the patch-2 branch November 11, 2019 18:01
@Mte90
Copy link
Contributor Author

Mte90 commented Nov 11, 2019

I hope that will be documented but thanks a lot!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants