-
Notifications
You must be signed in to change notification settings - Fork 566
Prototype for #1239, change slug for rest fields group #1240
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
Conversation
| public function __construct( CMB2 $cmb ) { | ||
| $this->cmb = $cmb; | ||
| self::$boxes[ $cmb->cmb_id ] = $this; | ||
| self::$rest_slug = 'cmb2'; |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
|
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 |
|
Yes it is only a prototype that works and not create issues with other metabox as it is right now. |
|
I appreciate the effort here. Will look into testing this as soon as I can. |
|
Well CMB2 is very important for my job since years so I am happy to contribute to it as much I can :-) |
|
any updates for this pr? |
|
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 |
|
Quite sad but I hope for a fix that works in all the cases :-) |
|
@Mte90 I just pushed a couple updates to the develop branch that will allow registering your own handlers for It's a bit more verbose than what you were trying to do, but it does work, and provides ultimate flexibility. |
|
I hope that will be documented but thanks a lot! |
Description
This pr enable with a new parameter in the metabox to change the
cmb2slug 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
cmb2is a default value.Testing procedure
Create a new metabox, enable
show_in_restand alsorest_group_valuesand define a slug.Probably we need a better parameter that is more explicative then this one about the purpose.
Checklist: