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

Prototype for #1239, change slug for rest fields group #1240

Open
wants to merge 3 commits into
base: develop
from

Conversation

Projects
None yet
3 participants
@Mte90
Copy link

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:

Mte90 added some commits Mar 12, 2019

@@ -117,6 +124,10 @@ public static function maybe_init_and_hookup( CMB2 $cmb ) {
public function __construct( CMB2 $cmb ) {
$this->cmb = $cmb;
self::$boxes[ $cmb->cmb_id ] = $this;
self::$rest_slug = 'cmb2';

This comment has been minimized.

Copy link
@slaFFik

slaFFik Mar 14, 2019

Collaborator

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

This comment has been minimized.

Copy link
@Mte90

Mte90 Mar 14, 2019

Author

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

This comment has been minimized.

Copy link
Member

commented Mar 14, 2019

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

This comment has been minimized.

Copy link
Author

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

This comment has been minimized.

Copy link
Member

commented Mar 14, 2019

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

@Mte90

This comment has been minimized.

Copy link
Author

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 :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.